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 force-pushed the rohit/popper-recommendation branch from ed5fd05 to 65cbee8 Compare March 26, 2021 05:09
@rohit2sharma95 rohit2sharma95 force-pushed the rohit/popper-recommendation branch from 65cbee8 to 624f645 Compare April 7, 2021 20:32
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 rohit2sharma95 force-pushed the rohit/popper-recommendation branch from 624f645 to 9d36147 Compare April 14, 2021 05:06
@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 rohit2sharma95 force-pushed the rohit/popper-recommendation branch from 9d36147 to 7cd45bd Compare April 14, 2021 05:21
@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.

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

Successfully merging this pull request may close these issues.

3 participants