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

Dropdown — Apply positioning only when Popper is not used #33482

Merged
merged 1 commit into from Apr 17, 2021

Conversation

rohit2sharma95
Copy link
Collaborator

@rohit2sharma95 rohit2sharma95 commented Mar 26, 2021

Apply positioning styles on the dropdown menus only when they are not using Popper. Since initial positioning styles may interfere with Popper (Previously these styles caused issues).

Ref:

Preview:- https://deploy-preview-33482--twbs-bootstrap.netlify.app/docs/5.0/components/dropdowns/

@rohit2sharma95 rohit2sharma95 requested a review from a team as a code owner March 26, 2021 05:02
@rohit2sharma95 rohit2sharma95 added this to Inbox in v5.0.0 via automation Mar 26, 2021
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

If I’m reading this right, this will now also require the correct data attribute for any placement, yes? We’ll want to document that in our Migration guide I think.

@thednp
Copy link

thednp commented Apr 12, 2021

You guys intend to keep the CSS for position variations of .dropdown-menu-end, .dropend, .dropup, .dropstart?

In the latest BETA3, the .dropup menu won't show above the target without Popper.js and the style for .dropdown-menu-end (mainly right:0; left: auto) is missing.

Or you intend to give Popper.js full power over the dropdown position and lock this plugin in this dependency?

@thednp
Copy link

thednp commented Apr 12, 2021

I would suggest you keep the following CSS in place, for minimal functionality without Popper:

.dropdown-menu-end {
  right: 0;
  left: auto;
}

.dropup .dropdown-menu {
  top: auto;
  bottom: 100%;
}

These two are missing and much needed.

@rohit2sharma95
Copy link
Collaborator Author

this will now also require the correct data attribute for any placement, yes?

@mdo these data attributes are added to the dropdown menu automatically when it is not positioned by Popper. (Done in #32986). So these are not required to be added by the user.

if (isDisplayStatic) {
Manipulator.setDataAttribute(this._menu, 'popper', 'static')
}

@rohit2sharma95
Copy link
Collaborator Author

Or you intend to give Popper.js full power over the dropdown position and lock this plugin in this dependency?

Popper is already required for Dropdown, although you can disable the dynamic positioning if you want static position only.

I would suggest you keep the following CSS in place, for minimal functionality without Popper:

.dropdown-menu-end {
  right: 0;
  left: auto;
}

.dropup .dropdown-menu {
  top: auto;
  bottom: 100%;
}

These styles are applied when the dropdown is not using Popper and these are not needed when Popper is used.

@thednp
Copy link

thednp commented Apr 14, 2021

Yes, as of beta 3, they're missing. I mean you set a "dropup", no Popper, it's gonna work like "dropdown".

As for dropdown-menu-end, this

.dropdown-menu-end {
    --bs-position: end;
}

does absolutely nothing.

v5.0.0 automation moved this from Inbox to Approved Apr 17, 2021
@mdo mdo merged commit 7100a0d into main Apr 17, 2021
v5.0.0 automation moved this from Approved to Done Apr 17, 2021
@mdo mdo deleted the rohit/popper-recommendation branch April 17, 2021 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v5.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants