melvin-bot[bot]
commented
9 months ago Job added to Upwork: https://www.upwork.com/jobs/~01230125fc5ec18c7d
Closed techievivek closed 7 months ago
melvin-bot[bot]
commented
9 months ago Job added to Upwork: https://www.upwork.com/jobs/~01230125fc5ec18c7d
melvin-bot[bot]
commented
9 months ago Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
melvin-bot[bot]
commented
9 months ago Platforms in OP are ✅)melvin-bot[bot]
commented
9 months ago Triggered auto assignment to @flaviadefaria (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.
melvin-bot[bot]
commented
9 months ago Current assignee @s77rt is eligible for the External assigner, not assigning anyone new.
techievivek
commented
9 months ago For context, we discovered and discussed about this issue here: https://expensify.slack.com/archives/C02NK2DQWUX/p1690561889854179
techievivek
commented
9 months ago @s77rt Can you confirm if this issue is reproducible on other platforms? or just on Android?
s-alves10
commented
9 months ago Main composer is displayed when editing two messages
There are several cases we should handle in this issue
After all, if there is no focused editing messages, main composer should be shown. If not, main composer should be hidden
We store the shouldShowComposeInput flag in Oynx store through the below function
The main composer will be shown when this function is called with true.
This function is called with true on unmount, and delete, and with false on focus.
On blur event, we add event listener for keyboardDidHide and show the composer after keyboard is hidden. So when we switch message B to edit mode, message A gets blurred and keyboardDidHide event listener added. So if we force hide the keyboard, main composer is shown. This is the root cause for case 1
When there are multiple messages in edit mode, if blur event on any element(with keyborad hidden) or save/delete action happens, the shouldShowComposeInput function will be called with true and the main composer will be displayed. This is the reason for case 2
EmojiPicker's active reportAction is changed only when emoji picker is opened. So even when the emoji picker is closed and other input gets focused, EmojiPicker's active reportAction isn't changed. This is the root cause for case 3
When we need to show or hide the main composer, remove the event listener added before. Doing this prevents the unexpected hide of main composer
Show composer only when the active message is canceled/saved or unmounted.
isFocused and check this value before showing composerReportActionContextMenu.isActiveReportAction but this won't work because we clear the active action early in modalDidHide. To solve this, don't clear the active action in modalDidHide and clear it in confirm/cancel callback. This way, we can control when to clear the active action of delete modal. We clear it when show confirm delete modal from context menu, and don't clear it when publishDraft is calledWhen a message in edit mode gets focused, clear the active action of the EmojiPicker. Doing this prevents the incorrect active reportAction of EmojiPicker and won't raise the race condition issue mentioned here
keyboardDidHide event listeners, add parameter called shouldShow to the openReportActionComposeViewWhenClosingMessageEdit function.import * as Composer from '../actions/Composer';
export default (shouldShow) => {
Composer.setShouldShowComposeInput(shouldShow);
};import {Keyboard} from 'react-native';
import * as Composer from '../actions/Composer';
let keyboardDidHideListener = null;
export default (shouldShow) => {
if (keyboardDidHideListener) {
keyboardDidHideListener.remove();
keyboardDidHideListener = null;
}
if (!shouldShow) {
Composer.setShouldShowComposeInput(false);
return;
}
if (!Keyboard.isVisible()) {
Composer.setShouldShowComposeInput(true);
return;
}
keyboardDidHideListener = Keyboard.addListener('keyboardDidHide', () => {
Composer.setShouldShowComposeInput(true);
keyboardDidHideListener.remove();
});
};And replace all ComposerActions.setShouldShowComposeInput with openReportActionComposeViewWhenClosingMessageEdit in ReportActionItemMessageEdit
onBlur event is called before the onPress handler is called. To solve this, we can prevent blurring on discard/save/emoji button clicks. To do this, add the following props to save, cancel, and emoji buttons
onMouseDown={(e) => e.preventDefault()}We can now safely remove the code below https://github.com/Expensify/App/blob/e555022ef97cec4ddbc39283c0d537d6366a2a49/src/pages/home/report/ReportActionItemMessageEdit.js#L332-L335
deleteDraft, call openReportActionComposeViewWhenClosingMessageEdit function with true only when we should show the main composer.Add clearActiveReportAction to the EmojiPickerAction and ReportActionContextMenu
Don't clear the active action in modalDidHide. Remove the following code
https://github.com/Expensify/App/blob/0196aa26543f6e877387510cec7b99549c8e4345/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js#L331
and update the context menu's showDeleteModal call here
if (closePopover) {
// Hide popover, then call showDeleteConfirmModal
hideContextMenu(false, () => showDeleteModal(reportID, reportAction, true, clearActiveReportAction, clearActiveReportAction));
return;
}
// No popover to hide, call showDeleteConfirmModal immediately
showDeleteModal(reportID, reportAction, true, clearActiveReportAction, clearActiveReportAction);onFocus handler of Composer here, add the following code
if (!EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) {
EmojiPickerAction.clearActiveReportAction();
}
if (!ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) {
ReportActionContextMenu.clearActiveReportAction();
}We clear the active report action when another composer gets focused
I think there won't be race condition you mentioned here
Add an parameter isFocused to the deleteDraft function.
In deleteDraft and unmount event, clear the active report action if the current message is an active action and show the main composer if the current message is active
In deleteDraft
if (isFocusedRef.current || EmojiPickerAction.isActiveReportAction(props.action.reportActionID) || ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) {
openReportActionComposeViewWhenClosingMessageEdit(true);
if (EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) {
EmojiPickerAction.clearActiveReportAction();
}
if (ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) {
ReportActionContextMenu.clearActiveReportAction();
}
}In unmount
if (!isFocusedRef.current && !EmojiPickerAction.isActiveReportAction(props.action.reportActionID) && !ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) {
return;
}
if (EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) {
EmojiPickerAction.clearActiveReportAction();
}
if (ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)) {
ReportActionContextMenu.clearActiveReportAction();
}isContextMenuActive state in ReportActionItem when ReportActionContextMenu.isActiveReportAction() changes. So replace the above code with const isActiveReportActionForMenu = ReportActionContextMenu.isActiveReportAction(props.action.reportActionID);
useEffect(() => {
setIsContextMenuActive(isActiveReportActionForMenu);
}, [isActiveReportActionForMenu]);deleteDraft is called. When get focused, setIsFocused(true) is called but isFocusedRef.current value doesn't change until this effect callback is called. The unmount handler is called just before this ref value is updated and so the handler returns here.In order to solve this issue, we need to sync the isFocused and isFocusedRef.current. So remove the above useEffect and set isFocusedRef.current whenever setIsFocused is called
This works for all cases as expected
s77rt
commented
9 months ago @techievivek Yes, on all platforms (as long as the window width is small). Just a note the second video in OP does not reflect the current state. https://github.com/Expensify/App/pull/23618 is not deployed to staging yet.
s77rt
commented
9 months ago @s-alves10 Thanks for the proposal. Your RCA is correct. The suggested solution although it looks like it would work it may cause a race condition ^1. The alternative solution looks more like a workaround (we are not clearing the report action in the right place).
It would better to fix the first solution race condition e.g. have ReportActionItemMessageEdit useEffect run before ReportActionItem or find another solution to https://github.com/Expensify/App/issues/22214.
s77rt
commented
9 months ago @s-alves10 Thanks for the update. I think the race condition fix may result in a regression
EmojiPicker is related to draft messages
It's related to the message itself as well e.g. you open the EmojiPicker to react to a message. If the message got deleted the picker should be hidden.
dukenv0307
commented
9 months ago Chat - Main composer is displayed when editing two messages when the first one is cancelled/save
After canceling or saving the draft message, we will display the main composer again by this logic https://github.com/Expensify/App/blob/024d210bc0b5a0454b62a72de60057af67cdd045/src/pages/home/report/ReportActionItemMessageEdit.js#L129
We only should display main composer if the number of the draft message of this report is 0
s77rt
commented
9 months ago @dukenv0307 Thanks for the proposal. I don't think your RCA is correct. This issue only occurs after opening the emoji picker. Can you please update your proposal taking this into account?
rojiphil
commented
9 months ago The Composer shows up even though the focus is still in message B. This happens when another message A had been edited before and had also used the Emoji Picker.
As already mentioned, the report action remains active even though the emoji picker is hidden. This causes the composer to show up.
To solve the problem, whenever the ReportActionItemMessageEdit is unmounted, we can safely hide the EmojiPicker if this is an active report action. We can replace this code as follows.
This will fix the current issue and #22214.
if (EmojiPickerAction.isActiveReportAction(props.action.reportActionID)) {
EmojiPickerAction.hideEmojiPicker(true);
}
// Skip if this is not the focused message so the other edit composer stays focused.
if (!isFocusedRef.current) {
return;
}@rojiphil Thanks for the proposal but isn't this proposed already by @s-alves10?
rojiphil
commented
9 months ago @rojiphil Thanks for the proposal but isn't this proposed already by @s-alves10?
But, he is also suggesting other things. If that is required, then, his proposal is a good choice. I do not think that is required. That way, my proposal is different. But, then, you can decide. I just provided my proposal.
techievivek
commented
9 months ago Hey, I have updated the issue description to include the original bug where the main composer gets focused when editing two messages.
👋 @garrettmknight coming from this bug https://github.com/Expensify/App/issues/17531#issuecomment-1662191253. This issue is linked to https://github.com/Expensify/App/issues/17531, which we decided to close due to the PR being reverted. Since the original issue is complex and was priced at $2000, I'll update this one to start with the same price.
melvin-bot[bot]
commented
9 months ago Upwork job price has been updated to $2000
techievivek
commented
9 months ago @s77rt
@techievivek Yes, on all platforms (as long as the window width is small). Just a note the second video in OP does not reflect the current state. https://github.com/Expensify/App/pull/23618 is not deployed to staging yet.
I see, in that case, can you please ensure the proposals include fix for both emoji picker and the race condition bug. Also, let me know if you want me to update anything in the OP, thanks.
s77rt
commented
9 months ago @rojiphil Thanks for the update. For that proposal I have already gave a review that it may result in a race condition.
s-alves10
commented
9 months ago @s77rt
Do we need to update the issue description? I think it should include https://github.com/Expensify/App/issues/17531 as well
s77rt
commented
9 months ago Yeah, we have 4 cases (at least) to handle:
Will have to summarise those before updating the OP, but feel free to post proposals
rojiphil
commented
9 months ago Yeah, we have 4 cases (at least) to handle:
@s77rt Thanks for the feedback and details. There is quite some history to this issue which I was unaware of. Will look at this more closely and see if I am able to propose something here.
s77rt
commented
9 months ago @techievivek Please update the description to:
Main compose box should not be displayed
Main compose box is displayed
s77rt
commented
9 months ago Notes:
Additional testing (regression) steps:
techievivek
commented
9 months ago @s77rt Updated, thanks.
s-alves10
commented
9 months ago @s77rt I made a slight changes considering the various cases. It works fine on web platform. I'll test on all platforms soon
s77rt
commented
9 months ago @s-alves10 Thanks for the update. I haven't read the proposal yet but at first glace looks like there is a lot of code :sweat_smile: It would be great if you can summarise the solution of each case in a sentence or two e.g. https://github.com/Expensify/App/issues/17531#issuecomment-1646614420. Regardless I will review asap. I'm also looking forward for platforms test.
s77rt
commented
9 months ago @s-alves10 Any update on the above?
s-alves10
commented
9 months ago Will update soon
s77rt
commented
9 months ago @s-alves10 Thanks for the update.
I think it's clear as we'd be reapplying the previous solution.
When we delete or unmount a message, we show the composer no questions asked
We should show the main composer only if we delete or unmount the active message. A message is considered active if:
ReportActionContextMenu.isActiveReportAction but as I understand this does not work as expected because once we delete the message we clear the report action (too early) and when we check isActiveReportAction it returns false, we deleted an active message but lost the info that tells us that - this will make composer not show when needed (regression). Can you explain how we are going to handle this scenario?Hold
s-alves10
commented
9 months ago @s77rt
I don't suggest to clear the report action on unmount or delete. I am suggesting to clear the report action when other report action gets focused. This is similar to setting isFocused to true on focus event.
s77rt
commented
9 months ago @s-alves10 I'm not saying that we should clear it. I'm saying it's already being cleared here right? So calling isActiveReportAction in ReportActionItemMessageEdit's useEffect may be too late.
s-alves10
commented
9 months ago @s77rt In my proposal, I suggested to manage confirm delete modal and context menu separately. We need to check report action for confirm delete modal when checking if we should show composer
s-alves10
commented
9 months ago @s77rt I misunderstood you. I think the above code should be deleted. We need to clear the reportActionID on focus event
s-alves10
commented
9 months ago @s77rt
I've tested all 4 cases mentioned above and works fine. Race condition won't be happen. I'll find more edge cases. By the way, can you tell me your thoughts on my solution?
s77rt
commented
9 months ago @s-alves10 I would really prefer not to add another state management for context menu, there should be only one, the confirm modal is supposed to be a part of the context menu.
I think the above code should be deleted. We need to clear the reportActionID on focus event
I think it's more logical to clear the report action when the context menu is hidden. There can be cases where we just hide the context menu and not focus on another report action.
s-alves10
commented
9 months ago @s77rt
Here are my thoughts.
I would really prefer not to add another state management for context menu, there should be only one, the confirm modal is supposed to be a part of the context menu.
We have some interactions related to the report action in edit mode. They are emoji picker and delete modal. We don't have context menu for actions in edit mode. So I wanted to separate context menu and delete modal, and to handle emoji picker and delete modal similarly. In addition, I think there is a big gap between context menu and delete modal. Why do they need to be handled as one state?
I think it's more logical to clear the report action when the context menu is hidden. There can be cases where we just hide the context menu and not focus on another report action.
I thought handling emoji picker and delete modal similarly would be good. For emoji picker, we have race condition if we clear the report action when it hides. So I suggest not to clear it on hide. I think similar race condition can be happened for delete modal as well because delete modal can be shown for report action not in edit mode similar to emoji picker. I think this consideration makes sense
These are just my humble opinion. Expecting your feedback
s77rt
commented
9 months ago @s-alves10 The delete modal is currently a part of ReportActionContextMenu they should have same state for the time being. I think the problem with that is timing right? (we need to make sure events go in a certain order) If this approach is getting too complicated it may be a good idea to take a step back and find another approach.
(Let's not worry about the emoji picker for now)
melvin-bot[bot]
commented
9 months ago 📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
rojiphil
commented
9 months ago 2. It has context menu open: Let's discuss this point.
@s77rt I was checking the various scenarios here but do not understand the use case when context menu is open. Are the steps to reproduce this mentioned somewhere? Please let me know. Thanks. Sorry if I missed this somewhere.
s77rt
commented
9 months ago @rojiphil Edit a message, clear the input then press the save button. This will show the delete modal which is a part of the context menu.
rojiphil
commented
9 months ago @rojiphil Edit a message, clear the input then press the save button. This will show the delete modal which is a part of the context menu.
Got it. Thanks. Investigating...
s-alves10
commented
9 months ago @s77rt
I think we can only one state for ReportActionContextMenu as you mentioned.
We can solve the case 2 as follows We should show the main composer only if we delete or unmount the active message. A message is considered active if:
ReportActionContextMenu.isActiveReportAction. Don't clear the reportAction for ReportActionContextMenu in modalHide. We clear it when an action gets focused. And we set reportAction when context menu is opened or delete modal is opened without clearing it. This way isActiveReportAction returns true when on unmount or deleteDraft callEmojiPicker is opened. We can check this using EmojiPickerAction.isActiveReportAction. Don't clear the reportAction for EmojiPickerAction as it is now. We set reportAction for EmojiPickerAction when EmojiPicker is opened, and clear it when any action gets focused.Note: I don't think we need to show the main composer in deleteDraft. It'll be unmounted for sure and we can add this code to the unmount handler only
Let me know your thought on this idea
s77rt
commented
9 months ago @s-alves10 Thanks for the update. For the most part I like the idea and the consistency how we handle both the emoji picker and context menu in the same way. Unfortunately we have a problem:
Don't clear the reportAction for ReportActionContextMenu in modalHide.
We can't just undo that because this was added here https://github.com/Expensify/App/pull/20185 to fix https://github.com/Expensify/App/issues/18165. We need to fix the original issue in another way before we can remove that code.
rojiphil
commented
9 months ago There are focus issues when multiple messages are been edited. Also, there are consistency issues with the show/hide of the Composer when multiple messages are edited. The scope is covered in the mentioned use cases in the problem description which needs to be addressed.
Note: Message Editor refers to the editor of a message which is a report action.
The current implementation hides the Composer when the Message Editor gets focus and shows the Composer whenever the Message Editor is blurred or draft is deleted. The Composer is also shown on unmount of the Message Editor if it is the currently focused one as shown here. The current approach relies on showing/hiding the Composer when the focus comes in and goes out of the Message Editor.
Hiding the Composer when Message Editor gets focused is the right thing to do. Then, the next logical question to ask is 'When do we show the Composer?' We are currently trying to handle this during the unmount here and on blur here. There, we are trying to identify all the cases where we should skip showing the Composer. Now, that would be a challenge as we have to target all individual cases. Further, showing/hiding the Composer will mount/unmount itself and will also bring/lose focus on the Composer. When there are multiple Message Editors and the focus needs to be on one of the Message Editors, this will bring a competition between Composer and Message Editor to gain focus. According to me, these are the problems that we need to address to resolve this problem.
This solution attempts to resolve the problem by remaining within the scope of ReportActionItemMessageEdit.js.
Along with isFocusedRef, this approach relies on states for ContextMenu and EmojiPicker which can be appropriately set/reset when context menu/Emoji Picker are shown/hidden.
The following code demonstrates the changes:
1) Maintain states for isContextMenuOpenRef and isEmojiPickerOpen [here]()
const [isContextMenuOpen, setIsContextMenuOpen] = useState(false);
const [isEmojiPickerOpen, setIsEmojiPickerOpen] = useState(false);
const isContextMenuOpenRef = useRef(false);
const isEmojiPickerOpenRef = useRef(false);
useEffect(() => {
isContextMenuOpenRef.current = isContextMenuOpen;
}, [isContextMenuOpen]);
useEffect(() => {
isEmojiPickerOpenRef.current = isEmojiPickerOpen;
}, [isEmojiPickerOpen]);2) Use these states during unmount to show the Main Composer as follows:
useEffect(() => {
return () => {
if ( isFocusedRef.current || isContextMenuOpenRef.current || isEmojiPickerOpenRef.current )
{
ComposerActions.setShouldShowComposeInput(true);
}
};
}, []);3) For ContextMenu, set and reset the state when it is shown/hidden here. We can call setIsContextMenuOpen(true) just before calling showDeleteModal and reset the state in the cancel and confirm callback handlers as shown below. As shown in the code, we can also call setIsFocused(true) during confirm callback as the focus is already lost due to the context menu. This will ensure that, during unmount, the Main Composer is shown.
setIsContextMenuOpen(true);
ReportActionContextMenu.showDeleteModal(props.reportID, props.action, false, () => {setIsContextMenuOpen(false); setIsFocused(true); deleteDraft()}, () => {setIsContextMenuOpen(false);InteractionManager.runAfterInteractions(() => textInputRef.current.focus());});4) For EmojiPicker, we can reset the state when it is hidden here.
onModalHide={() => {
+ setIsEmojiPickerOpen(false);Currently, we do not have a way to set the state for EmojiPicker. For this, we can add a prop onModalShown here and set the state within this.
onModalShown={() => {
setIsEmojiPickerOpen(true);
}}
We also need to call onModalShown when the EmojiPicker is shown here in this code
if(props.onModalShown) props.onModalShown();
5) Additionally,
a) We can remove these lines as deleteDraft will always unmount the Message Editor
b) Remove this code here
I think, this is simple enough to handle Case 2 and Case 3 and to keep all regressions at bay.
This proposal is an alternate approach. The proposed solution is to move the decision of where to put focus at a later point when Composer is mounted or props get updated. Further, instead of identifying all individual cases where we should skip showing the Composer, let us show the composer only when it is needed.
The implementation can be as follows:
1) We know that there can be only editor (i.e. Composer or Message Editor) which can gain focus. To ensure that the appropriate editor gets focus, let us introduce a callback focusActionItemCallback for Message Editor just like we have focusCallback here for Composer. Further, we can update onComposerFocus, focus and clear to accommodate focusActionItemCallback in ReportActionComposeFocusManager.js. Also, we can add a helper function isCurrentActionItem to identify if the Message Editor is the currently focused one.
2) To use focusActionItemCallback, let us first call onComposerFocus for the Message Editor when it gains focus here. Additionally, we can clear the callback for Message Editor whenever a) Composer gains focus here and b) Message Editor is unmounted (addressed in point-4). Now, whenever focus gets called, Composer or Message Editor gets focused depending on the current use case.
3)
a) Now, let us call focus at the required places. First, when the Composer is mounted, we can replace this.focus() here with ReportActionComposeFocusManager.focus(). Also, the same can be done for update here. By making the decision on where to focus at the time of mount/update of Composer, we can avoid competition between Composer and Message Editor for gaining focus.
b) Also, we can remove these lines as deleteDraft will always unmount the Message Editor and we will handle this case in point-4 below.
4) As mentioned earlier, let us show the composer only when it is needed. Now, we know that the only case when we need to show the composer is if the currently focused Message Editor is been unmounted. We can leverage the recently added helper function isCurrentActionItem for this purpose. Also, when the Message Editor unmounts, we need to bring the focus back to the right place (i.e. Composer or another Message Editor). We can do this by replacing this code with the following:
useEffect(() => {
return () => {
if (ReportActionComposeFocusManager.isCurrentActionItem(onFocusCallback))
{
ReportActionComposeFocusManager.clear(true);
ComposerActions.setShouldShowComposeInput(true);
}
ReportActionComposeFocusManager.focus();
};
}, []);5) Case 2 and Case 3 in the problem statement should get resolved with the above implementation. Regarding Case 1, the keyboardDidHide handler triggers the problem. For me, Case 1 gets fixed by doing just like the web version here. I am not clear why the keyboardDidHide handler was added in the first place. Or may be I am missing some information here due to internal discussions. @s77rt , would you know the issue that will get reproduced if we do not use the keyboardDidHide handler?
Alternative Solution to new focus Management:
Purpose:
a) Remove the usage of ReportActionComposeFocusManager.focus() from the main proposal (Solution 2) as there is no real reason to call this. Reference comment here
b) Solve the edge case in mWeb i.e. Main Composer does not show up when EMojiPicker is already displayed for an unfocused message input and the same message input is deleted from the remote device. Reference comment here
Note: Usage of isCurrentActionItem is still very much there as mentioned in the main proposal.
The following changes to the main proposal above are proposed:
a) We can remove the ReportActionComposeFocusManager.focus() in Point(4) of proposal. The updated code can be like this:
useEffect(() => {
return () => {
if (ReportActionComposeFocusManager.isCurrentActionItem(onFocusCallback))
{
ReportActionComposeFocusManager.clear(true);
ComposerActions.setShouldShowComposeInput(true);
}
};
}, []);
b) Replace this code with the following. The inline comment is self-explanatory:
return () => {
// Show the main composer if this message is deleted from another client
// but has emoji picker already displayed for this message.
// This is to prevent the main composer stays hidden until we switch to another chat.
if (EmojiPickerAction.isActiveReportAction(props.action.reportActionID) ) {
ComposerActions.setShouldShowComposeInput(true);
}
}; c) Point (3a) in the proposal is not required. Further, no need to accommodate focusActionItemCallback in focus() as mentioned in point (1).
Note: In the case of ContextMenu, whether it is Remote Deletion or Local Deletion, the focusActionItemCallback will refer to this message input. So, the unmount logic of ReportActionItemMessageEdit will kick in to show the Main Composer with the help of isCurrentActionItem even without the need of ReportActionComposeFocusManager.focus(). So, the alternative solution mentioned here holds good for the Context Menu.
rojiphil
commented
9 months ago @s77rt I have tested the proposal across platforms and it seems to work well. Please do share your thoughts on the proposal. Also, let me know if there are any additional cases that you would like me to test. Thanks.
s77rt
commented
9 months ago @rojiphil Thanks for the proposal. If I understood the concept, we will replace the "focused" state management: instead of using the isFocused() method we will be using our own. We will set the flag as focused in the native focus callback but we won't unset it in the native blur callback instead we will unsent it when we focus on another input or delete that input. Would that be about right?
Also the focus callback or focusActionItemCallback is not needed here, right?
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Case 1:
Case 2:
Case 3:
Notes:
Additional testing (regression) steps:
Expected Result:
Main compose box should not be displayed
Actual Result:
Main compose box is displayed
Workaround:
Don't edit two messages at once.
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.47
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/93399543/232604613-64146c4c-9b6c-47c3-bbe3-839122ff6006.mp4
https://github.com/Expensify/App/assets/35863227/00f9d5e3-6bbf-44f3-a668-6d6dff8a1c83
Expensify/Expensify Issue URL:
Issue reported by: @s77rt
Slack conversation: https://expensify.slack.com/archives/C02NK2DQWUX/p1690561889854179
View all open jobs on GitHub
Upwork Automation - Do Not Edit