-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix focus loss after message deletion in edit mode #49538
base: main
Are you sure you want to change the base?
Fix focus loss after message deletion in edit mode #49538
Conversation
@brunovjk, It seems the linting error in
|
Good work so far @Shahidullah-Muffakir. Good question, I'll ask on Slack about it and get back here. Thanks. |
@brunovjk, my bad about the earlier message. I got it wrong. The linting error is actually in the same file where we made our PR changes. We will modify
To:
|
Great @Shahidullah-Muffakir! I'm waiting for someone to comment on Slack, if nothing happens by Monday, please feel free to make the necessary updates and then we'll hand it over to an internal reviewer. What do you think? Thanks. |
@brunovjk, Sounds good to me. We'll wait until Monday for any comments on Slack. Thank you! |
@Shahidullah-Muffakir Do you also see a delay to focus the composer? Thanks. Screen.Recording.2024-09-21.at.11.45.50.movChorme: Screen.Recording.2024-09-21.at.11.46.42.movIn Screen.Recording.2024-09-21.at.11.55.15.movI wonder if in native it should Focus after deleting a message, according to the issue "Web/Desktop - Chat - Focus is lost ..." no, what do you think @youssef-lr? Thank you. |
@brunovjk, Yes, I see the same delay in focusing the composer. It seems like the delay might be caused by the time it takes to process the message deletion request. I believe this line of code could be the reason for the delay:
Not sure if there’s much we can do about this, Thank you. |
Details
This PR fixes a problem where deleting a message while editing causes the text input area (composer) to lose focus. The issue was caused by a mistake in how a callback function was assigned, which led to unexpected behavior. The solution is to call the runAndResetCallback function directly to make sure it works correctly and resets as needed. Also, I corrected some typos in the references to onConfirmDeleteModal to avoid any future errors.
Fixed Issues
$#45472
PROPOSAL:#45472 (comment)
Tests
Offline tests
Same as tests.
QA Steps
same as tests.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
This feature is only supported on the desktop app and web.
Android: mWeb Chrome
This feature is only supported on the desktop app and web.
iOS: Native
This feature is only supported on the desktop app and web..
iOS: mWeb Safari
This feature is only supported on the desktop app and web.
MacOS: Chrome / Safari
safari.mov
MacOS: Desktop
desktop.mov