Skip to content

Invisible view on FAB using refactored modal fix #57536

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 27, 2025
Merged

Invisible view on FAB using refactored modal fix #57536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a35e10
fix: invisible view on FAB
BartoszGrajdek Feb 27, 2025
a9d5a33
Merge remote-tracking branch 'origin/main' into @BartoszGrajdek/FAB-i…
BartoszGrajdek Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/components/Modal/BottomDockedModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import noop from 'lodash/noop';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import type {ViewStyle} from 'react-native';
import {BackHandler, DeviceEventEmitter, Dimensions, KeyboardAvoidingView, Modal, View} from 'react-native';
import {BackHandler, DeviceEventEmitter, Dimensions, InteractionManager, KeyboardAvoidingView, Modal, View} from 'react-native';
import {LayoutAnimationConfig} from 'react-native-reanimated';
import useThemeStyles from '@hooks/useThemeStyles';
import getPlatform from '@libs/getPlatform';
Expand Down Expand Up @@ -41,6 +41,7 @@ function BottomDockedModal({
const [isTransitioning, setIsTransitioning] = useState(false);
const [deviceWidth, setDeviceWidth] = useState(() => Dimensions.get('window').width);
const [deviceHeight, setDeviceHeight] = useState(() => Dimensions.get('window').height);
const handleRef = useRef<number>();
Copy link
Contributor

@ZhenjaHorbach ZhenjaHorbach Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting fix
And sorry for the stupid question 😅
But could you explain in more detail where the invisible view came from
And how it fixed it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

react-native-modal library which this code originates from was originally using InteractionManager, but during my refactor it caused some problems so I removed it, which didn't create any regressions.

The problem was that recently a change was made in E/App here that caused the FAB to rely specifically on the InteractionManager in our modals. Since we didn't have it anymore in the refactored version this caused the issue the QA team has found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After adding the InteractionManager back I've tested if any of the problems I faced before also appeared, but it seems like they may have been happening due to some other bugs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh
Now I get it
Thanks for the detailed explanation !


const styles = useThemeStyles();

Expand Down Expand Up @@ -108,11 +109,13 @@ function BottomDockedModal({

useEffect(() => {
if (isVisible && !isContainerOpen && !isTransitioning) {
handleRef.current = InteractionManager.createInteractionHandle();
onModalWillShow();

setIsVisibleState(true);
setIsTransitioning(true);
} else if (!isVisible && isContainerOpen && !isTransitioning) {
handleRef.current = InteractionManager.createInteractionHandle();
onModalWillHide();

setIsVisibleState(false);
Expand All @@ -133,12 +136,18 @@ function BottomDockedModal({
const onOpenCallBack = useCallback(() => {
setIsTransitioning(false);
setIsContainerOpen(true);
if (handleRef.current) {
InteractionManager.clearInteractionHandle(handleRef.current);
}
onModalShow();
}, [onModalShow]);

const onCloseCallBack = useCallback(() => {
setIsTransitioning(false);
setIsContainerOpen(false);
if (handleRef.current) {
InteractionManager.clearInteractionHandle(handleRef.current);
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #61986, we have a case that clearInteractionHandle not work as expected, and we handled it here #62023

}
if (getPlatform() !== CONST.PLATFORM.IOS) {
onModalHide();
}
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/sidebar/FloatingActionButtonAndPopover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ function FloatingActionButtonAndPopover({onHideCreateMenu, onShowCreateMenu, isT
fromSidebarMediumScreen={!shouldUseNarrowLayout}
animationInTiming={CONST.MODAL.ANIMATION_TIMING.FAB_IN}
animationOutTiming={CONST.MODAL.ANIMATION_TIMING.FAB_OUT}
shouldUseNewModal
menuItems={menuItems.map((item) => {
return {
...item,
Expand Down
Loading