Dropdown in Modal Doesn't Work

[davilera]

Describe the bug
When a modal has a dropdown as one of its child components, the dropdown doesn't work as expected:

Modal with a button to open the dropdown.

Dropdown is now open, but the user can't see it (notice the scrollbars).

The dropdown is inside the modal.

To reproduce
Create the following simple component:

const ModalWithBrokenDropdown = () => (
  <Modal title="Dropdown in Modal Test">
    <Dropdown
      renderToggle={ ( { onToggle } ) => (
        <Button isDefault onClick={ onToggle } >Open Dropdown</Button>
      ) }
      renderContent={ () => ( <p>Hi there!</p> ) }
    />
  </Modal>
);

and render it. Then, click on the button and you'll see the error.

Expected behavior
The dropdown should be properly positioned below the button and overlay the modal.

Desktop (please complete the following information):

• OS: Linux
• Browser: Firefox 70 and Chromium 77

Additional context

• WordPress 5.3.2

[epiqueras]

I can't reproduce this. Where are you rendering the modal?

[davilera]

I'm rendering it in a custom page... If, however, I do this in Gutenberg:

registerPlugin( 'modal-with-broken-dropdown', {
  render: ModalWithBrokenDropdown,
} );

I get this:

which looks quite nice. However, I'm still wondering: shouldn't the dropdown be aligned with the button?

[epiqueras]

I rendered it in Gutenberg directly by modifying the source code.

It sounds like a stylesheet missing/order issue.

[davilera]

It's not about CSS, @epiqueras. I think this might have something to do with what @ItsJonQ said on Slack...

If you look at the rendered HTML in Gutenberg, the Dropdown is rendered in #editor > .components-drop-zone__provider > .components-navigate-regions > .edit-post-layout, whereas the Modal is rendered in... the root, I think.

If you now look at my original code, the Dropdown is rendered inside the Modal. So, is this related to React Portals or something? I'm completely lost, here :-(

[ItsJonQ]

@epiquera+ @davilera

Hallo 👋!

My guess is that there's something up with the Popover slot, which Dropdown uses internally.

In this example, the Dropdown's Popover isn't able to find the Slot (which should be at the root(ish) level of Gutenberg). As a fallback, the Popover isn't Portal'ed, but rather, rendered in place, trapping it within the Modal 😅

As for Button alignment... that part is most likely a CSS issue (my gut tells me something display related)

[epiqueras]

@davilera your custom page probably doesn't have a popover slot.

[ItsJonQ]

@davilera Would you be able to try adding a <Popover.Slot /> somewhere and see if that helps?

Example implementation:
https://github.com/WordPress/gutenberg/blob/master/storybook/stories/playground/index.js#L55

[davilera]

@epiqueras it doesn't.

@ItsJonQ so I tried this:

const ModalWithBrokenDropdown = () => (
  <div>
    <Modal title="Dropdown in Modal Test">
      <Dropdown
        renderToggle={ ( { onToggle } ) => (
          <Button isDefault onClick={ onToggle } >Open Dropdown</Button>
        ) }
        renderContent={ () => ( <p>Hi there!</p> ) }
      />
    </Modal>
    <Popover.Slot />
  </div>
);

but didn't work.

And this doesn't work either:

...
  <SlotFillProvider><Popover.Slot /></SlotFillProvider>
...

[ItsJonQ]

@davilera Interesting!! Hmm... 🤔

What about:

<SlotFillProvider>
  <Modal>
    ...
  </Modal>
  <Popover.Slot />
</SlotFillProvider>

[davilera]

@davilera Interesting!! Hmm... thinking

What about:

<SlotFillProvider>
  <Modal>
    ...
  </Modal>
  <Popover.Slot />
</SlotFillProvider>

@ItsJonQ Genius! That did the trick :-) So I guess I'll have to add the SlotFillProvider and Popover.Slot in the root of my page for this to work... but that's OK, I can live with that.

There are two issues missing, though:

1. What about the position of my Popover? If I make the Modal bigger, the problem accentuates:

and, as far as I can tell, the Popover's position is set automatically (no CSS involved).

2. Should we create a PR and update the Popover's documentation, so that other developers know they (may) need a SlotFillProvider in their app?

[ItsJonQ]

the Popover's position is set automatically (no CSS involved)

@davilera Yes! You are correct. The positioner uses DOM element numbers and MATH to figure how where to put things. However, CSS rendering the Button and it's container may have effects on the calculation numbers (not your fault at all).

I'm not sure what's happening exactly, but I feel like the detection and calculations should be more reliable and resilient.

I too have experienced mispositioned Popovers (like your screenshot), but only when using outside of Gutenberg/WordPress.

Should we create a PR and update the Popover's documentation

That's a great suggestion 👍 !

I'm going to look into this a bit more. In the meantime, you're more than welcome to submit a PR to add that documentation. You'll most likely get to that before I do 😉

[ItsJonQ]

@davilera Hai there! I just created a CodeSandbox test to double-check the rendering and positioning of the components.

It looks like it's working okay for me. Would you be able to double-check on your end?

https://codesandbox.io/s/wp-components-dropdown-modal-test-fjx2j

The code I used is roughly the same as the examples above.

Thanks!

[davilera]

Thanks for your help, @ItsJonQ.

I just created a CodeSandbox test to double-check the rendering and positioning of the components.

Interesting... I tried to remove all my CSS (and its dependencies with WordPress) and, not surprisingly, this is what I get:

a broken dialog, as wp-components' style is not enqueued. If I enqueue it, I get the proper dialog, but the issue remains:

I see that in your CodeSandbox, you're using the styles defined in @wordpress/components, which got me thinking... I'm using the styles packages in WordPress itself. What if I install the Gutenberg plugin? And here's what I get:

So, yup, it looks like a future update of WordPress core packaging a newer Gutenberg version will fix the issue. However, is there anything we need to do about it? Do we need to let someone know?

you're more than welcome to submit a PR to add that documentation

Sure! I'll give it a shot ;-)

[ItsJonQ]

So, yup, it looks like a future update of WordPress core packaging a newer Gutenberg version will fix the issue

Phew! That's good! It's a bummer that it's currently not working as expected in the current release. Glad to know it's working on the latest! Hopefully, the fix will be released soon 🙏

Do we need to let someone know?

I'm not sure 😊 . @youknowriad ? Heh

[youknowriad]

So, yup, it looks like a future update of WordPress core packaging a newer Gutenberg version will fix the issue. However, is there anything we need to do about it? Do we need to let someone know?

Gutenberg will be merged entirely for the next WordPress release. So nothing needed here I think.

[youknowriad]

In the Gutenberg context though, I wonder if there's a way we can tweak our component tree to avoid the need to explicitly add the slot in the plugin tree.

#19453 (comment) This should ideally just work.

[epiqueras]

In the Gutenberg context though

The error was in a custom plugin page, not in Gutenberg.

[davilera]

@youknowriad, @epiqueras is right: my issue occurred in a custom page—in Gutenberg it worked as expected.

However, I detected yet another problem: Tab doesn't work as expected in a dropdown inside a modal, as it doesn't cycle through the elements in the dropdown.

https://codesandbox.io/s/wp-components-dropdown-modal-test-z07zs

[epiqueras]

I added the a11y feedback label.

[MarcoZehe]

However, I detected yet another problem: Tab doesn't work as expected in a dropdown inside a modal, as it doesn't cycle through the elements in the dropdown.

Note that I am blind and cannot see the screenshots. Is that dropdown some kind of menu or similar in behavior or purpose to an html:select element? If so, tab is not supposed to nove through each of these items, which in an html:select would be html:option items. Or on the Mac, part of the popup menu that opens if you click such a Select. The way to navigate those via the keyboard is by arrow up and down. Similar to the dropdown menu of the main or formatting toolbars, for example, for Tools, More Formatting, etc. Tab is supposed to go to the next element inside a group of controls. So to the next select, button, input, etc. So if you have some kind of dropdown menu open, it is expected that tab doesn't move among those items.

[davilera]

Hi @MarcoZehe. Thanks for joining the discussion.

Originally I reported an incompatibility issue between two components: a Modal and a Dropdown inside the modal. This Dropdown is a visual component that pops up when the user clicks on a certain element. For instance, if you're using Gutenberg, it's used in several places: when you click on the "add block" button, the list of available blocks shows up in a Dropdown. Another example: when you click on the scheduled date of a post, the "date time picker" also appears in a Dropdown.

@epiqueras added the «Needs Accessibility Feedback» label because we encountered another issue: if this Dropdown component contains multiple fields (such as, for example, two text controls, or a date time picker), the user can't "jump" from one field to the other using Tab.

Moreover, I've just identified yet another issue. Dropdowns are supposed to "disappear" when the user clicks outside them. If the Dropdown is part of a Modal, clicking outside the Dropdown and the Modal works as expected: the Dropdown disappears. But, if the user clicks outside the Dropdown but inside the Modal, the Dropdown remains... very weird.

[MarcoZehe]

Thanks for the explanation, @davilera :-) If a dropdown indeed consists of multiple fields, these should definitely be tabbable. However, in accessibility terms, a "dropdown" like the calendar picker is considered more another modal inside such a modal. Since you should either make a selection or dismiss that with escape. So, there is definitely a distinction between dropdowns in the sense of a dropdown menu, like the list of block types, or a calendar picker. The latter should definitely be tabbable, since it has multiple fields for day, month, year, and even the time of day.

[MarcoZehe]

In addition, scheduled posts did seem to work in the past, like when opening the date picker from the Publicize panel. I did an advent calendar last month, and there, I could definitely tab through the different items of the calendar picker. It behaved a bit odd in that it allowed to tab outside that popup, and closed it at some point automatically, and set the proper date and time, but it definitely allowed tabbing. Has that broken in the standard Gutenberg now? Uh-Oh because there were some bigger changes in #18779 and #19235 which adjusted tabbing behavior.

[davilera]

The latter [a calendar picker] should definitely be tabbable, since it has multiple fields for day, month, year, and even the time of day.

Well, then that's the problem. When using the calendar picker in a Dropdown that's in a Modal, it doesn't work.

I did an advent calendar last month, and there, I could definitely tab through the different items of the calendar picker.

Indeed. As far as I can tell, the problem only occurs when the calendar picker is used in this "complex" setup: a calendar picker inside a dropdown inside a modal (inside the page)...

[ItsJonQ]

Wow! Thanks for the thoughts + feedback @davilera + @MarcoZehe .

It's great that something that originally appeared to be a superficial CSS issue, uncovered something deeper.

I think this (pretty complex) issue of handling multi-layered UI is common. It usually stems from the UIs not being aware of each other. They're only self-aware.

This is usually solvable by adjusting an internal mechanism they all share. Something that's responsible for layering + closing.

That way, it will know how to close (via click or ESC press) things in order.

[spencerfinnell]

Likely related: #8468

[ItsJonQ]

@diegohaz Is working on a refactor for SlotFill. A component that's not directly responsible for the updates we'll need for correct Portal sequencing, but it'll play a role:

#19242

[diegohaz]

#19453 (comment) This should ideally just work.

This is the same "issue" which caused tooltips to have low contrast in disabled buttons when used outside Gutenberg: #19337 (comment)

Maybe Popover should fallback to some default portal node instead of being added as a child?

[epiqueras]

Maybe Popover should fallback to some default portal node instead of being added as a child?

That sounds like a good idea.

[talldan]

@davilera, @epiqueras, @ItsJonQ It'd be great if the title/description of the issue could be updated to summarise what was discovered. Also if there are any other separate parts like documentation updates, could they be split out into separate issues?

At the moment, this issue is quite hard for a dev to pick up as they'd have to read through quite a complicated exchange of comments.

[mirka]

I'm going to close this for now since much of the implementation has changed in the past few years, and the problems I saw from a quick skim of this thread do not reproduce anymore.

Like @talldan said, please split out separate issues if there are still any remaining problems. Thanks!