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

Add support for dropdown container to be inline-block (#8559) #16491

Closed
wants to merge 1 commit into from

Conversation

jffng
Copy link
Contributor

@jffng jffng commented Jul 9, 2019

Description

This change adds a prop to the Dropdown component that allows a user to set the containing element to display inline-block. This allows the popover to appear centered relative to its trigger in instances outside of the Gutenberg context.

How has this been tested?

Tested in the Gutenberg playground and passes respective unit tests.

Screenshots

Before
Screen Recording 2019-07-09 at 05 37 PM

After
Screen Recording 2019-07-09 at 05 36 PM

Types of changes

Fixes #8559

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@youknowriad
Copy link
Contributor

youknowriad commented Nov 18, 2019

While triaging the repository I found this PR. Sorry for the delay here, we should have taken a look earlier.

--

Why did you decide on a prop here? Should this be the default behavior of the Dropdown instead?

@jffng
Copy link
Contributor Author

jffng commented Nov 18, 2019

Hello!

Why did you decide on a prop here?

I used a prop as a convenience, giving the consumer of the component a method to handle an expected use case (display the popover aligned to the button), while not changing the default behavior.

Should this be the default behavior of the Dropdown instead?

I'm not sure - I would need to look more thoroughly throughout the repo to see where and how the Dropdown component is used, and what effects the change would have.

@youknowriad
Copy link
Contributor

So I went ahead and included this change in #18631 in addition to other dropdown changes. Thanks a lot for your contribution

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

Successfully merging this pull request may close these issues.

Dropdown: Popover content is wrong positioned
2 participants