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

[EuiContextMenu] Popover has to close before changes made to items are reflected #2579

Closed
lou00011 opened this issue Dec 2, 2019 · 11 comments

Comments

@lou00011
Copy link

lou00011 commented Dec 2, 2019

I wanted to make a context menu where each individual menu element can be toggled. I am doing this by changing the icon from empty to check.

        <EuiContextMenuItem
            key='1'
            icon={state?  'check' : 'empty'}
            onClick={e =>  setState(!state))
        >Entry A</EuiContextMenuItem>

However, I found that the context menu doesn't reflect the changes on the fly, but the popover must close and reopen for the change to be reflected.

Is intentional? Is there anyway I can make this element reactive like everything else?

@lou00011 lou00011 changed the title ContextMenuItem has to close before change is reflected Popover has to close before changes made to ContextMenuItem are reflected Dec 2, 2019
@lou00011
Copy link
Author

lou00011 commented Dec 2, 2019

Digging deeper, it seems ContextMenuPanel is whats preventing item passed into it being updated unless the popover reopens. This is related to #710, which is fixed (?)

@chandlerprall
Copy link
Contributor

#710 is related, but it doesn't fix this instance.

@cchaos
Copy link
Contributor

cchaos commented Dec 2, 2019

@lou00011 It seems that if you're just doing selection, you could use EuiSelectable inside of an EuiPopover. There's an example of this concept in the Sizing and containers section of http://localhost:8030/#/forms/selectable

@lou00011
Copy link
Author

lou00011 commented Dec 2, 2019

@lou00011 It seems that if you're just doing selection, you could use EuiSelectable inside of an EuiPopover. There's an example of this concept in the Sizing and containers section of http://localhost:8030/#/forms/selectable

I have solved the problem by taking all items out of ContextMenuPanel and put them directly into the popover. The overall look isn't affected.

@cchaos cchaos added ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible and removed ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible labels Mar 27, 2020
@shrey
Copy link
Contributor

shrey commented Jun 12, 2020

@cchaos Is this issue relevant still? if yes, what all changes do you expect?

@chandlerprall
Copy link
Contributor

chandlerprall commented Jun 12, 2020

Here's a code sandbox demonstrating the issue - https://codesandbox.io/s/competent-williams-z29jd?file=/index.js

When the popup is open, 4 items are shown. The first two use EuiContextMenuItem directly, and tie their check icon to a state variable. The second two use EuiContextMenuItem wrapped in another component, and their icons are tied to a localized state variable. Clicking on either of the last two items toggles that individual item's item as expected. However, clicking on either of the top two items has no apparent effect, until the popover is closed & re-opened, at which point the new state takes effect.

A couple notes:

  • the wrapping popover does not appear to matter, other than providing a way of "refreshing" the menu items
  • does not matter how many times the first two items are clicked, closing & re-opening the popover always shows the state was toggled exactly once

Definitely acts like something is cloned, or put in a state; however, I don't see either of those concepts in use at a quick glance. This is an odd bug for sure, feel free to ask for help digging through it if you get stuck!

@cchaos cchaos changed the title Popover has to close before changes made to ContextMenuItem are reflected [EuiContextMenu] Popover has to close before changes made to items are reflected Sep 20, 2020
@spalger
Copy link
Contributor

spalger commented Nov 12, 2020

+1 attempting to have a context menu item name change when clicked. I'm using <EuiContextMenu> with the panels prop, changing the items in panel 0, verified that the render is occuring, but the items don't reflect the changes until I close and reopen the popover.

@cchaos
Copy link
Contributor

cchaos commented Nov 12, 2020

I'd like to try to better understand the use case for not automatically closing the popover upon selection in the context menu.

The most common use-case, is using it as a menu system. Where the options trigger an action elsewhere, similar to a right click menu or an application menu.

Screen Recording 2020-11-12 at 09 54 30 AM

Another use-case is to make a single-selection type of menu, like pagination. Where the result of the selection is displayed elsewhere but it also only needs to indicate the current selection the first time it pops up.

Screen Recording 2020-11-12 at 09 58 10 AM

--

If the use-cases that are needed here is to make a multi-select dropdown list, we highly suggest the use of EuiSelectable within a regular EuiPopover. The mechanics are such that it does a much better job at relaying selection including accessibility.

If this is not the use-case, please explain further. It'll be much easier for us to understand and help solve.

@spalger
Copy link
Contributor

spalger commented Nov 12, 2020

That's ultimately what I ended up implementing, toggle the state and then close the popover, and it's a better user experience for this use case. I still think it's a bug that the panels/items don't propagate properly when the <ContextMenu> is rerendered.

Only examples I can think of where this is would really be necessary are

  • you want to show the context menu immediately, but some of the values need to be loaded asynchronously
  • you want to show more details in a content menu which are updated over time

2020-11-12 14 38 20

@p-young
Copy link

p-young commented Mar 19, 2022

I'm also running into this issue.

My use case is having a theme selector toggle in the context menu. When I toggle it, the accompanying icon and label do not change (but when closed and re-open, show the change).

Is there any plan to fix this or is it just suggested to not use context menu and re-make it using EuiPopover and other components?

@cee-chen
Copy link
Contributor

This appears to have been resolved at some point (React 18 changes? some other set of bug fixes?) - I can't reproduce Chandler's above CodeSandbox code in latest EUI main.

If anyone in this thread is still encountering this issue on latest EUI, please open a new issue with a minimum reproducible sandbox.

@cee-chen cee-chen closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2023
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

7 participants