Skip to content

Add .dropdown-menu-dark#30171

Merged
XhmikosR merged 6 commits into
mainfrom
dropdown-menu-dark
Sep 24, 2020
Merged

Add .dropdown-menu-dark#30171
XhmikosR merged 6 commits into
mainfrom
dropdown-menu-dark

Conversation

@mdo
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 mdo force-pushed the dropdown-menu-dark branch from a2438de to e13be2e Compare June 19, 2020 04:31
@mdo
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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?

@mdo mdo force-pushed the dropdown-menu-dark branch from e13be2e to c5fc9e1 Compare September 21, 2020 23:59
Copy link
Copy Markdown
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
Copy Markdown
Contributor

@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
Copy Markdown
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 <xhmikosr@gmail.com>
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