Skip to content
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: Edit composer moves focus to main composer after press LHN menu item #24481

Merged
merged 16 commits into from
Aug 22, 2023
Prev Previous commit
Next Next commit
move to existing cleanup func
  • Loading branch information
tienifr committed Aug 20, 2023
commit 3a64c6ecc6ca866747c0e4ab66246788982d6729
19 changes: 12 additions & 7 deletions src/components/Composer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import Text from '../Text';
import isEnterWhileComposition from '../../libs/KeyboardShortcut/isEnterWhileComposition';
import CONST from '../../CONST';
import withNavigation from '../withNavigation';
import ReportActionComposeFocusManager from "../../libs/ReportActionComposeFocusManager";
import willBlurTextInputOnTapOutside from "../../libs/willBlurTextInputOnTapOutside";
import ReportActionComposeFocusManager from '../../libs/ReportActionComposeFocusManager';
import willBlurTextInputOnTapOutside from '../../libs/willBlurTextInputOnTapOutside';

const propTypes = {
/** Maximum number of lines in the text input */
Expand Down Expand Up @@ -161,7 +161,7 @@ function Composer({
}) {
const textRef = useRef(null);
const textInput = useRef(null);
const willBlurTextInputOnTapOutsideRef= useRef();
const willBlurTextInputOnTapOutsideRef = useRef();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, why did you change this to a ref?

Copy link
Contributor Author

@tienifr tienifr Aug 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to just directly call the function instead of creating the ref, right? I've updated that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we do it that way in reportActionCompose.

Copy link
Contributor

@Ollyws Ollyws Aug 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But do we even need it allall seeing as we are only adding this to composer on web, and willBlurTextInputOnTapOutside only returns true on web. Correct me if I'm wrong but won't it return true 100% of the time here?

Copy link
Contributor Author

@tienifr tienifr Aug 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree! I was trying to replicate the conditions from ReportActionCompose without noticing that.

willBlurTextInputOnTapOutsideRef.current = willBlurTextInputOnTapOutside();
const initialValue = defaultValue ? `${defaultValue}` : `${value || ''}`;
const [numberOfLines, setNumberOfLines] = useState(numberOfLinesProp);
Expand Down Expand Up @@ -358,10 +358,13 @@ function Composer({
updateNumberOfLines();
}, [updateNumberOfLines]);

useEffect(() => () => {
ReportActionComposeFocusManager.clear();
ReportActionComposeFocusManager.focus();
}, []);
useEffect(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this still here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My fault!!

() => () => {
ReportActionComposeFocusManager.clear();
ReportActionComposeFocusManager.focus();
},
[],
);

useEffect(() => {
// we need to handle listeners on navigation focus/blur as Composer is not unmounting
Expand All @@ -379,6 +382,8 @@ function Composer({
}

return () => {
ReportActionComposeFocusManager.clear();
ReportActionComposeFocusManager.focus();
unsubscribeFocus();
unsubscribeBlur();
document.removeEventListener('paste', handlePaste);
Expand Down