[web][fix] Fixed cursor issue in the message editing
ClosedPublic
Actions

Authored by • kuba on May 31 2023, 12:39 AM.

Details

Reviewers

michal
inka
ashoat

Commits

rCOMM9461175282dc: [web][fix] Fixed cursor issue in the message editing

Summary

There was a bug, where after typing the character the cursor was moved to the end of the text area. It made editing the middle of the content very cumbersome. This address this issue, now focusAndUpdateText() method is called only once, not after every text change.

Raised in https://linear.app/comm/issue/ENG-3981/[web]-fix-cursor-moving-to-the-end-of-the-textbox-after-character

Test Plan

Entered edit mode on the web
Checked if the textarea is focused
Edited the message in the middle, checked if the cursor is not moved to the end

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• kuba created this revision.May 31 2023, 12:39 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMay 31 2023, 12:39 AM

• kuba edited the summary of this revision. (Show Details)May 31 2023, 12:40 AM

• kuba edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B19918: Diff 27292.May 31 2023, 12:58 AM

• kuba requested review of this revision.May 31 2023, 12:58 AM

ashoat added inline comments.May 31 2023, 5:26 AM

web/chat/chat-input-text-area.react.js
69 ↗	(On Diff #27292)	Would it be bad to add `updateHeight` to the dep list here?

• kuba added inline comments.May 31 2023, 6:52 AM

web/chat/chat-input-text-area.react.js

69 ↗

(On Diff #27292)

This results in this warning thrown all the time:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.

ashoat added inline comments.May 31 2023, 7:25 AM

web/chat/chat-input-text-area.react.js
69 ↗	(On Diff #27292)	Can you investigate why adding `updateHeight` to the dep list causes a stack overflow? In particular, I'd like to understand why `onChangePosition` is getting changed when the height changes

• kuba marked an inline comment as done.Jun 1 2023, 2:53 AM

• kuba added inline comments.

web/chat/chat-input-text-area.react.js
69 ↗	(On Diff #27292)	I believe that this is why it causes a stack overflow: `updateHeight()` depend on `onChangePosition()` which calls `updateDimensions()` in `edit-text-message.react.js`. `updateDimensions()` then recalculates the position of the background edit box and updates the edit state by calling `updatePosition()`. When the edit state is updated, the `updatePosition` (in `edit-message-provider.js`) is recalculated, causing the recalculation of `updateDimensions()` which is passed to `onChangePosition()`.

ashoat accepted this revision.Jun 1 2023, 11:05 AM

ashoat added inline comments.

web/chat/chat-input-text-area.react.js
69 ↗	(On Diff #27292)	Makes sense! Can you add this code comment for clarity? (Make sure lines are <=80 chars each) // We want to update the height when the text changes. We can't include // updateHeight in the dep list because it causes a stack overflow... see // comments on D8035 for more details