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 .dropdown-menu-dark #30171

Merged
merged 6 commits into from
Sep 24, 2020
Merged

Add .dropdown-menu-dark #30171

merged 6 commits into from
Sep 24, 2020

Conversation

mdo
Copy link
Member

@mdo mdo commented Feb 14, 2020

@mdo mdo requested a review from a team as a code owner February 14, 2020 01:37
@mdo mdo mentioned this pull request Feb 14, 2020
@MartijnCuppens
Copy link
Member

I'm a bit worried that once we're adding some dark components, people will keep asking for support for other elements (eg. modals, pagination (which I declined in the past, #29892), form elements, ect... )

Once we switch to CSS variables, it 'll be way easier to colorize components.

@mdo
Copy link
Member Author

mdo commented Feb 14, 2020

Yeah, I agree on that concern, but we can still say no to most.

I mostly wanted to do this because of how prominent our dark navbar component is. Completing the style there and connecting the two components (navbar and dropdown) makes a lot of sense to me. The rest, I'm not as sold on, though we might as well discuss our options further beyond this.

@MartijnCuppens
Copy link
Member

Ok, fine by me. Could you also switch the dropdowns of the dark navbars then (make sure to tackle the examples too)?

@mdo mdo changed the base branch from master to main June 16, 2020 20:11
@mdo
Copy link
Member Author

mdo commented Jun 19, 2020

Could you also switch the dropdowns of the dark navbars then (make sure to tackle the examples too)?

Know I +1ed this back in February, but coming back to this I don't know how much I want to actually replace the other dark navbars. I think since this is a modifier class, it can stay as an option documented under the dropdowns.

Any aversions to merging as-is without further examples or docs examples?

@MartijnCuppens
Copy link
Member

Any aversions to merging as-is without further examples or docs examples?

Now that we have CSS custom properties, this might be a good use case to use them. I think in we 'll need to update our other components to CSS custom properties (where suitable), adding this like might cause double work.

@mdo
Copy link
Member Author

mdo commented Sep 14, 2020

Thinking I'll leave this as-is for now and then come back in minor releases to CSS-variable-ify everything. Thoughts on that approach @MartijnCuppens?

Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

Job well done, @mdo!

Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

LGTM

@XhmikosR
Copy link
Member

I like it, I just don't know how we are going to proceed with dark components for all components in the future, but we'll discuss it later :)

@XhmikosR XhmikosR merged commit 585b3ec into main Sep 24, 2020
@XhmikosR XhmikosR deleted the dropdown-menu-dark branch September 24, 2020 15:55
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
* Add .dropdown-menu-dark

* Match background color to navbar dark

* Update docs to include a navbar example

* Update dropdowns.md

Co-authored-by: XhmikosR <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dropdown-dark
4 participants