[IS-3218] Fixed cursor issue on edit comment box #3434

aliabbasmalik8 · 2021-06-08T16:23:45Z

Details

Fixed Issues

Fixes #3218

Tests

QA Steps

Navigate to a conversation in e.cash
Hover over an owned message and click on the edit comment option in the menu
or
Tap up arrow key to edit the previous comment.
See the cursor is set to the end of the message.

Tested On

Screenshots

Web

Screen.Recording.2021-06-08.at.9.13.46.AM.mov

Mobile Web

Desktop

Screen.Recording.2021-06-08.at.9.22.54.AM.mov

iOS

Screen.Recording.2021-06-08.at.9.09.10.AM.mov

Android

Screenrecorder-2021-06-08-21-25-56-100_0_COMPRESSED.mp4

parasharrajat · 2021-06-09T08:42:50Z

src/pages/home/report/ReportActionItemMessageEdit.js

+     *
+     * @param {Event} e
+     */
+    onSelectionChange(e) {


Overall looks good but this state change is unnecessary, I have already seen a drawback of this and I am migrating tis unnecessary state change. So my suggestion here would be to remove this as I mentioned over the issue.
cc: @Gonals

@parasharrajat can you please let me know about the issue?
also, without state change, this issue will not be fixed.

This change is going against our old pattern of not making the Component controlled.

OSBotify · 2021-06-09T09:13:09Z

🚀 Deployed to staging in version: 1.0.65-6🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

kavimuru · 2021-06-09T15:11:44Z

mWeb - No cursor appears in edit comment box

Expected Result:

The cursor is set to the end of the message.

Actual Result:

No cursor appears in edit comment box

Actions Performed:

Go to https://staging.expensify.cash
Log in with applause account or gmail account
Navigate to a conversation in e.cash
Hover over an owned message and click on the edit comment option in the menu or tap up arrow key to edit the previous comment.

Platform:

iOS
mWeb ✔️
Android

Build:

1.0.66-0

Notes/Images/Video:

Bug5104994_Screen_Recording_20210609-103956_Chrome.mp4

parasharrajat · 2021-06-09T15:15:57Z

@kavimuru It's being fixed #3441.

kavimuru · 2021-06-09T16:06:48Z

App crashes when editing a comment

Expected Result:

User is able to edit and save the message successfully

Actual Result:

App keeps crashing when editing the message

Actions Performed:

Launch the app and login
Hover over an owned message and click on the edit comment option in the menu
Start editing the message

Platform:

iOS
mWeb
Android ✔️

Build:

1.0.66-0

Notes/Images/Video:

Bug5105065_20210609_112537_1_.mp4

expensify.txt

Julesssss · 2021-06-11T16:18:00Z

I believe this is no longer a deployBlocker, as this fix was merged and is on staging. Removing the label from here and the deploy issue.

parasharrajat · 2021-06-11T17:27:10Z

@Julesssss just a heads up, I feel that the change we have done here has unnecessary state updates. We just want to set the cursor at the end of the text when we initiate the edit. So why we are re-rendering it on every input and setting the cursor, making it a controlled input. I believe we are following uncontrolled Textinput throughout the app. Even here you can see at this line
https://github.com/Expensify/Expensify.cash/blob/19ad75ea6b6905ce31f37b5e8251d98e23add5a1/src/pages/home/report/ReportActionItemMessageEdit.js#L65
&
https://github.com/Expensify/Expensify.cash/blob/19ad75ea6b6905ce31f37b5e8251d98e23add5a1/src/pages/home/report/ReportActionItemMessageEdit.js#L120.
we don't use value prop intentionally.

So I think I can submit a PR to replace the changes here with

    componentDidMount() {
        this.textInput.setNativeProps({selection: {start: this.props.draftMessage.length, end: this.props.draftMessage.length}});
    }

What are your thoughts?

Julesssss · 2021-06-11T18:00:20Z

Thanks @parasharrajat, I need to review this on Monday but I think you make a good point. Have set a reminder to return to this.

aliabbasmalik8 · 2021-06-13T22:46:43Z

@Julesssss
We need a cursor position(selection) to add emojis in mid of the text. (there is an issue open to adding emojis functionality in the edit text compose box)

But for the 2nd point, we can use Direct Manipulation to avoid rerending also we can use this approach for the comment compose box.

Thanks

OSBotify · 2021-06-15T16:54:13Z

🚀 Deployed to production in version: 1.0.68-4🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

parasharrajat · 2021-06-15T16:57:23Z

@Julesssss did you get chance to look at #3434 (comment) ?

Julesssss · 2021-06-15T17:02:15Z

Ah, sorry I haven't yet. That makes sense to me though, good spot.

Would you like to raise an issue for this, and we can get it exported?

parasharrajat · 2021-06-15T17:07:55Z

It's fine. Just a small change. Let me know PR will be up in no time.

Julesssss · 2021-06-15T17:14:24Z

I'll try to add a small bonus to an existing contract instead. Thanks @parasharrajat

parasharrajat · 2023-06-02T23:06:22Z

src/pages/home/report/ReportActionItemMessageEdit.js


        this.state = {
            draft: this.props.draftMessage,
+            selection: {


Setting selection without focusing the input internally focuses the input on Safari which is then shifts to other elements causing bugs such as #15105

[IS-3218] Fixed cursor issue on edit comment box

d530a94

aliabbasmalik8 requested a review from a team as a code owner June 8, 2021 16:23

MelvinBot requested review from Gonals and removed request for a team June 8, 2021 16:24

Gonals approved these changes Jun 9, 2021

View reviewed changes

Gonals merged commit 5ead572 into Expensify:main Jun 9, 2021

OSBotify mentioned this pull request Jun 9, 2021

Deploy Checklist: Expensify.cash 2021-06-08 #3450

Closed

49 tasks

parasharrajat suggested changes Jun 9, 2021

View reviewed changes

isagoico added the DeployBlockerCash This issue or pull request should block deployment label Jun 9, 2021

roryabraham mentioned this pull request Jun 9, 2021

Set the focus on Edit Box after edit action. #3441

Merged

5 tasks

aliabbasmalik8 mentioned this pull request Jun 10, 2021

[IS-3218] Fixed cursor issue #3510

Merged

5 tasks

Julesssss removed the DeployBlockerCash This issue or pull request should block deployment label Jun 11, 2021

Jag96 mentioned this pull request Jun 16, 2021

Android - Crash when editing a message #3599

Closed

parasharrajat reviewed Jun 2, 2023

View reviewed changes

parasharrajat mentioned this pull request Jun 2, 2023

[HOLD for payment 2023-06-02] [$4000] mWeb - Chat - Reopening Edit option highlights the Delete option in a blue frame #15105

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IS-3218] Fixed cursor issue on edit comment box #3434

[IS-3218] Fixed cursor issue on edit comment box #3434

aliabbasmalik8 commented Jun 8, 2021 •

edited

Loading

parasharrajat Jun 9, 2021

aliabbasmalik8 Jun 9, 2021

parasharrajat Jun 11, 2021

OSBotify commented Jun 9, 2021

kavimuru commented Jun 9, 2021 •

edited by isagoico

Loading

parasharrajat commented Jun 9, 2021

kavimuru commented Jun 9, 2021

Julesssss commented Jun 11, 2021 •

edited

Loading

parasharrajat commented Jun 11, 2021

Julesssss commented Jun 11, 2021

aliabbasmalik8 commented Jun 13, 2021

OSBotify commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

Julesssss commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

Julesssss commented Jun 15, 2021

parasharrajat Jun 2, 2023

[IS-3218] Fixed cursor issue on edit comment box #3434

[IS-3218] Fixed cursor issue on edit comment box #3434

Conversation

aliabbasmalik8 commented Jun 8, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

parasharrajat Jun 9, 2021

Choose a reason for hiding this comment

aliabbasmalik8 Jun 9, 2021

Choose a reason for hiding this comment

parasharrajat Jun 11, 2021

Choose a reason for hiding this comment

OSBotify commented Jun 9, 2021

kavimuru commented Jun 9, 2021 • edited by isagoico Loading

mWeb - No cursor appears in edit comment box

Expected Result:

Actual Result:

Actions Performed:

Platform:

Build:

Notes/Images/Video:

parasharrajat commented Jun 9, 2021

kavimuru commented Jun 9, 2021

App crashes when editing a comment

Expected Result:

Actual Result:

Actions Performed:

Platform:

Build:

Notes/Images/Video:

Julesssss commented Jun 11, 2021 • edited Loading

parasharrajat commented Jun 11, 2021

Julesssss commented Jun 11, 2021

aliabbasmalik8 commented Jun 13, 2021

OSBotify commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

Julesssss commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

Julesssss commented Jun 15, 2021

parasharrajat Jun 2, 2023

Choose a reason for hiding this comment

aliabbasmalik8 commented Jun 8, 2021 •

edited

Loading

kavimuru commented Jun 9, 2021 •

edited by isagoico

Loading

Julesssss commented Jun 11, 2021 •

edited

Loading