Dropdown remove redundant code #35157
Conversation
| @@ -254,13 +251,7 @@ class Dropdown extends BaseComponent { | |||
| } | |||
|
|
|||
| const popperConfig = this._getPopperConfig() | |||
| const isDisplayStatic = popperConfig.modifiers.find(modifier => modifier.name === 'applyStyles' && modifier.enabled === false) | |||
There was a problem hiding this comment.
@mdo @rohit2sharma95: we need your input on this. There must some reason this was added?
There was a problem hiding this comment.
It identifies the behavior of the dropdown is static or not. Since the dropdown may be static without having the display: 'static' in the config or in the data attributes (Via the popper modifiers)
This flag variable is then used to add a data attribute which helps in the CSS to place the dropdown menu properly 🙂
See #32986 for the reference
| @@ -254,13 +251,7 @@ class Dropdown extends BaseComponent { | |||
| } | |||
|
|
|||
| const popperConfig = this._getPopperConfig() | |||
| const isDisplayStatic = popperConfig.modifiers.find(modifier => modifier.name === 'applyStyles' && modifier.enabled === false) | |||
There was a problem hiding this comment.
It identifies the behavior of the dropdown is static or not. Since the dropdown may be static without having the display: 'static' in the config or in the data attributes (Via the popper modifiers)
This flag variable is then used to add a data attribute which helps in the CSS to place the dropdown menu properly 🙂
See #32986 for the reference
js/src/dropdown.js
Outdated
| // Totally disable Popper for Dropdowns in Navbar | ||
| if (this._inNavbar) { | ||
| if (this._inNavbar || this._config.display === 'static') { |
There was a problem hiding this comment.
Doing this will fully disable the Popper for the dropdown (with display: 'static' in the config) and will not create a popper instance for the dropdown 😨
There was a problem hiding this comment.
If I have understand right, 'static' option as you say too, is given to "help the CSS to place the dropdown menu properly "
So in other words to 'disable' popper calculate things, right?
Our used css selector is [data-bs-popper]. So we do not miss something
Although, I am not sure if we still need popper initiated for another reason, as it seems useless.
Or we maybe could enable popper static to navbar too. These are too different approaches, but with the same result.
There was a problem hiding this comment.
@rohit2sharma95 kept the popper instance as you suggested 😃
GeoSot
force-pushed
the
gs/dropdown-clean-up
branch
from
f9226b5
to
cee526c
Compare
GeoSot
force-pushed
the
gs/dropdown-clean-up
branch
from
cee526c
to
6028e37
Compare
XhmikosR
force-pushed
the
gs/dropdown-clean-up
branch
from
d5754c3
to
5d50700
Compare
…ing static popper with no styles attached
XhmikosR
force-pushed
the
gs/dropdown-clean-up
branch
from
5d50700
to
6e30821
Compare
This MR,
data-bs-display="static"&isNavBarcase handlingdataApiKeydownHandlersimplifying the checks, and shows that a code block was uselessNOTE for reviewers:
Better review it, commit by commit. It will help you with the proper message and will guide you with sanity to follow the logic