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

Menu onDismiss on web called automatically. #3716

Closed
amin79 opened this issue Mar 1, 2023 · 20 comments
Closed

Menu onDismiss on web called automatically. #3716

amin79 opened this issue Mar 1, 2023 · 20 comments

Comments

@amin79
Copy link

amin79 commented Mar 1, 2023

I have a menu in my project. it works as expected on mobile devices and all browsers on my MacBook. But on any browsers in windows, when I click on it, it blinks a horizontal scroll view at the bottom of the screen and does not work. I found out that onDismiss is called there. When I commented it out, it works . But cannot close it then.

"react-native-paper": "^5.2.0",
"expo": "^48.0.4",
"react-native": "0.71.3",

@amin79 amin79 added the bug label Mar 1, 2023
@amin79
Copy link
Author

amin79 commented Mar 1, 2023

I found the problem. It comes from Menu.Item

When I use an object for the label in Menu.Item, it does not work. But if I just put a string or index in it, it works.
I used Text instead of Menu.Item and it works!!!

        {menuItems.map((item, index) => (
          <Text key={index}>{item.label}</Text>
          // <Menu.Item
          //   key={index}
          //   onPress={() => {
          //     closeMenu();
          //   }}
          //   title={item.label}
          // />
        ))}

@amin79
Copy link
Author

amin79 commented Mar 1, 2023

Something else that I found is that when I wrap the text inside Pressable, it has the same behaviour as Menu.Item

{menuItems.map((item, index) => (
          <>
            <Pressable
              key={item.id}
              style={{
                marginHorizontal: 10,
                marginVertical: 5,
                justifyContent: "center",
                height: 40,
              }}
              onPress={closeMenu}
            >
              <Text>{item.label}</Text>
            </Pressable>
            <Divider />
          </>        
        ))}

I don't understand what is going wrong here.

@github-actions
Copy link

github-actions bot commented Mar 1, 2023

Hey! Thanks for opening the issue. Can you provide a minimal repro which demonstrates the issue? Posting a snippet of your code in the issue is useful, but it's not usually straightforward to run. A repro will help us debug the issue faster. Please try to keep the repro as small as possible. The easiest way to provide a repro is on snack.expo.dev. If it's not possible to repro it on snack.expo.dev, then you can also provide the repro in a GitHub repository.

@danb4r
Copy link

danb4r commented Mar 9, 2023

@amin79, did you find a solution? I run into the same error here with react-native-paper 4.12.5..

@amin79
Copy link
Author

amin79 commented Mar 9, 2023

@danb4r no. It was very strange. It even sometimes didn’t work with some special texts! I used this library and it worked perfectly in all platforms.

@danb4r
Copy link

danb4r commented Mar 10, 2023

Thank you @amin79. I found the issue! I was missing the <Appbar> enclosing tag before <Appbar.Header>.

The example of <Appbar.Header> from the docs do not state that there must be a opening <Appbar> tag, they start the example straight with <Appbar.Header>: https://callstack.github.io/react-native-paper/4.0/appbar-header.html

So, just do and everything will work fine.. no auto dismiss and no screen overlapping:

<Appbar>
  <Appbar.Header>
    ...
  </Appbar.Header>
</Appbar>

@danb4r
Copy link

danb4r commented Mar 12, 2023

Well.. it turned out that the issue persists when using <Menu> alone.

Please find here a video of the issue https://github.com/danb4r/bugreport-paper1/blob/master/Repro.mp4?raw=true
and the code to repro it https://github.com/danb4r/bugreport-paper1

@lukewalczak , I am experiencing it together with the issue #3678 as reported.

@lukewalczak
Copy link
Member

Hey @danb4r, I've cloned your repo and try to reproduce the issue, however without the success:

menu_web

Could you please provide more details about your environment or repro steps (maybe I missed something during my check)?

@danb4r
Copy link

danb4r commented Mar 12, 2023

Oh.. let's go down the rabbit hole.

I just made a test with Firefox browser here and it works fine. The issue just occurs with Chrome browsers. Probably all of them.. Chrome, Brave and Edge confirmed.

My env:

expo-env-info 1.0.5 environment info:
System:
OS: Windows 10 10.0.22621
Binaries:
Node: 16.18.1 - C:\Program Files\nodejs\node.EXE
npm: 9.6.1 - C:\Program Files\nodejs\npm.CMD
SDKs:
Android SDK:
API Levels: 30, 31, 32, 33
Build Tools: 30.0.2, 30.0.3, 31.0.0, 33.0.0
System Images: android-30 | Intel x86 Atom_64, android-30 | Google APIs Intel x86 Atom, android-30 | Google APIs Intel x86 Atom_64, android-30 | Google Play Intel x86 Atom
IDEs:
Android Studio: AI-213.7172.25.2113.9123335
npmPackages:
@expo/webpack-config: ^18.0.1 => 18.0.1
babel-preset-expo: ^9.2.2 => 9.3.0
expo: ~48.0.6 => 48.0.6
react: 18.2.0 => 18.2.0
react-dom: 18.2.0 => 18.2.0
react-native: 0.71.3 => 0.71.3
react-native-web: ^0.18.12 => 0.18.12
Expo Workflow: managed

@lukewalczak
Copy link
Member

I just made a test with Firefox browser here and it works fine. The issue just occurs with Chrome browsers. Probably all of them.. Chrome, Brave and Edge confirmed.

As you can notice on the gif attached above I was testing it on the Chrome browser, however, I'll check it on the other browsers in my dedicated time.

@danb4r
Copy link

danb4r commented Mar 13, 2023

As you can notice on the gif attached above I was testing it on the Chrome browser, however, I'll check it on the other browsers in my dedicated time.

Thank you. I just tested on a clean Xubuntu/Linux VM and got the same previous results: on Firefox, it works nicely, on Chromium it automatically calls onDismiss and overflows.

Linux vm-xubuntu 5.15.0-58-generic #64-Ubuntu SMP Thu Jan 5 11:43:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Distributor ID: Ubuntu
Description: Ubuntu 22.04.2 LTS
Release: 22.04
Codename: jammy

Firefox version 110.0.1 (64-bit)
Chromium Version 111.0.5563.64 (Official Build) snap (64-bit)

@github-actions
Copy link

Hello 👋, this issue has been open for more than a month without a repro or any activity. If the issue is still present in the latest version, please provide a repro or leave a comment within 7 days to keep it open, otherwise it will be closed automatically. If you found a solution or workaround for the issue, please comment here for others to find. If this issue is critical for you, please consider sending a pull request to fix it.

@ErionTp
Copy link

ErionTp commented May 27, 2023

I have a menu in my project. it works as expected on mobile devices and all browsers on my MacBook. But on any browsers in windows, when I click on it, it blinks a horizontal scroll view at the bottom of the screen and does not work. I found out that onDismiss is called there. When I commented it out, it works . But cannot close it then.

"react-native-paper": "^5.2.0", "expo": "^48.0.4", "react-native": "0.71.3",

This problem happens when the height of the list or menu is bigger than the height of the screen, in this case, the menu can't find space and closes by itself. A workaround is putting your items in a list, and set a fixed height of your list but not bigger than the screen. I have the same issue in edge and chrome for windows

@danb4r
Copy link

danb4r commented Jul 21, 2023

This bug is still haunting everyone that uses the menu to the right side of the screen. On the left side of the screen it works as expected, but on the right side when on the web, it just closes by itself for no reason as soon as it is drawn on the screen.

This is my tempo fix.. and by "tempo" I really mean "tempo". I found a reasonable amount of time, 400ms, that has been shown to be pretty safe to ignore menu dismisses. It is an ugly hack but it just works. And may not be as ugly as this bug.

I beg to the developers to try to find the reason of this bug and fix it.

const [menuVisible, setMenuVisible] = useState(false);
const [menuFired, setMenuFired] = useState(0);

const openMenu = () => {
  setMenuVisible(true);
  setMenuFired(Date.now());  // sets last time menu was fired
};

const closeMenu = () => {
  // refuses to close menu in a period of time shorter than 400ms
  if (Date.now() - menuFired > 400) setMenuVisible(false);
};

@jjspierx
Copy link

jjspierx commented Oct 4, 2023

I have also experienced this bug. I have experienced it with RNP v4 and now with V5. The bug is definitely dependent on the location of the menu and the current width/height of the screen. For my use case I have a settings menu that is attached a button at the top right of the screen. This button shows an email address with a drop-down caret at large screen sizes and becomes a hamburger button at smaller screens. This causes the location of the menu to change at different screen sizes. For me, the menu was working with the full sized button, but failing when the screen size made the anchor point a hamburger menu. I think possibly it was forcing the menu to onto the vertical scrollbar on the right edge, and maybe that was causing it to dismiss? Not sure, but this bug definitely is still occurring as v5.10.6.

The temporary fix above by @danb4r is working fine for me for now. Thanks for that posting that.

@Rubenvdveen
Copy link

Rubenvdveen commented Oct 11, 2023

I also have experienced this bug. However the strange thing is that this does not occur on MacOS using Chrome for me. It only happens on Windows with Chrome. Somehow that makes a difference apparently?

Also, it's not related to the menu being on the right side of the screen for me, this also happens with menu's in the middle or left side of the screen. It probably does have to do with what @ErionTp said. I think it's because the menu would overflow outside of the screen size and that causes it to close by itself. Which is a huge problem ofcourse.

The menu should just find the max height from it's anchor position and use that instead. This issue is very large because an un-openable menu is obviously UX breaking for a lot of apps. Please look into this ASAP.

@julian-hecker
Copy link

Still experiencing this bug in 5.11.1. What could be going on here?

@anstokes
Copy link

anstokes commented Dec 5, 2023

It would appear to be related to the implementation of the animation in the Chromium browser (i.e., Chrome/Edge) on the Windows platform.

Adding the overflow: 'hidden' class to the wrapper fixes the scrollbar issue, but I also needed to reset the top and left variable state in the 'hide' method; otherwise it only worked on the initial show.

react-native-paper/src/components/Menu/Menu.tsx:386

-        this.setState({ menuLayout: { width: 0, height: 0 }, rendered: false });
+        this.setState({ top: 0, left: 0, menuLayout: { width: 0, height: 0 }, rendered: false });

react-native-paper/src/components/Menu/Menu.tsx:669

     position: 'absolute',
+    overflow: 'hidden'

@lewibs
Copy link

lewibs commented Jan 9, 2024

Why is this issue closed? It still exists in the current deployment.

Context: Edge
Version: "react-native-image-picker": "^7.0.2"
Code:

export default function Header({
  navigation,
  back,
}) {
  const [visible, setVisible] = React.useState(false);
  const openMenu = () => {
    console.log("open");
    if (!visible) setVisible(true)
  };
  const closeMenu = () => {
    console.log('close')
    if (visible) setVisible(false)
  };
  const sesh = useAuthClient().useSesh();
  const logout = useAuthClient().useLogout();
  const gotoNext = useGotoNext();

  React.useEffect(()=>{
    if (initialized === false) {
      initialized = true;
      gotoNext();
    }
  }, []);

  return (
    <Appbar.Header
      //elevated={true}
      style={{
        //elevation:zindex.front,
        backgroundColor:(sesh.state === role.CARETAKER)?colors.success.main:colors.primary.main
      }}
    >
        {back ? <Appbar.BackAction onPress={navigation.goBack} /> : null}
        <Appbar.Content title={<Image src={logoDefault} width={150}/>} />
        {(sesh.state === role.CARETAKER)?<Text style={{padding:margin.large}}>Clocked in</Text>:undefined}
        {!back ? (
            <Menu
                visible={visible}
                onDismiss={closeMenu}
                anchor={
                    <Appbar.Action
                    icon="dots-vertical"
                    onPress={openMenu}
                    />
                }
            >
              <Menu.Item
                  onPress={() => {logout.mutate()}}
                  title="Logout"
              />
            </Menu>
        ) : null}
    </Appbar.Header>
  );
}

As mentioned above this issue disappears when removing Menu.Item and replacing it with text.

@lukewis
Copy link

lukewis commented Jan 12, 2024

@lewibs - I had the same problem, which brought me here. After some more digging, it looks like this issue is now being tracked in #3854 . I suggested a workaround in that thread. Hope it helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants