Skip to content

Feature/accept all colours#506

Merged
tcbegley merged 25 commits intodbc-team:masterfrom
shwinnn:feature/accept-all-colours
Jan 19, 2021
Merged

Feature/accept all colours#506
tcbegley merged 25 commits intodbc-team:masterfrom
shwinnn:feature/accept-all-colours

Conversation

@shwinnn
Copy link
Copy Markdown
Contributor

@shwinnn shwinnn commented Jan 11, 2021

Allows the user to pass any valid css colour to the "color" property of components, as well as the bootstrap built in colours as before.

@shwinnn shwinnn requested a review from tcbegley January 11, 2021 17:36
@shwinnn
Copy link
Copy Markdown
Contributor Author

shwinnn commented Jan 12, 2021

Added some tests.

Copy link
Copy Markdown
Collaborator

@tcbegley tcbegley left a comment

Choose a reason for hiding this comment

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

This is looking good. A few comments:

  1. Can we kill the package-lock.json file? The diff is unusually large (were you using Yarn?) and I normally wait until making a release to update it in any case to minimise noise in the commit history.
  2. The fill colour on hover with dbc.Button(..., outline=True) doesn't get updated, it stays as secondary (default). This is probably hard to fix because you can't set hover styles inline.
  3. Related to last point, should we perhaps steer people towards default colours in Button? It's not just the outline style that suffers, but also the hover effects for regular buttons get destroyed by the override here. I basically am unsure we should make it easier for people to make their app look worse 😆

Anyway, other than that it's all looking pretty good! Here's the app i was testing with for reference. Note that I chose the colours randomly for testing haha.

import dash
import dash_bootstrap_components as dbc
import dash_html_components as html

app = dash.Dash(external_stylesheets=[dbc.themes.BOOTSTRAP])

app.layout = html.Div(
    [
        dbc.NavbarSimple(brand="Navbar", color="red"),
        dbc.Navbar("Hello", color="blue"),
        dbc.Container(
            [
                dbc.Alert("Hello", color="chartreuse"),
                dbc.Button("Hi", color="rebeccapurple"),
                dbc.Button("There", outline=True, color="cyan"),
                dbc.Badge("Badge", color="magenta"),
                dbc.Label("Testing", color="orange"),
                dbc.Progress(
                    color="chocolate", value=50, striped=True, animated=True
                ),
                dbc.Spinner(color="darkviolet"),
                dbc.Card("This is a card", body=True, color="deepskyblue"),
                dbc.DropdownMenu(
                    [dbc.DropdownMenuItem("Item 1")],
                    label="Toggle",
                    color="lightcoral",
                ),
                dbc.FormText("Text", color="lightsteelblue"),
                dbc.ListGroup(
                    [
                        dbc.ListGroupItem("Item 1", color="fuchsia"),
                        dbc.ListGroupItem(
                            dbc.ListGroupItemText("Text", color="chocolate")
                        ),
                    ]
                ),
            ],
            className="p-5",
        ),
    ]
)

if __name__ == "__main__":
    app.run_server(debug=True)

@shwinnn
Copy link
Copy Markdown
Contributor Author

shwinnn commented Jan 13, 2021

I've sorted out the problems with hovering at the cost of 2 new requirements. Bundle size only goes up to 302KB though. What do you think?

@shwinnn
Copy link
Copy Markdown
Contributor Author

shwinnn commented Jan 14, 2021

Reverted the hover colours, button is back to only allowing bootstrap contextual colours.

Copy link
Copy Markdown
Collaborator

@tcbegley tcbegley left a comment

Choose a reason for hiding this comment

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

This looks great. If you could just put the Button test back in (looks like it got deleted in the back and forth on that component) I think this is ready to go.

Comment thread src/components/__tests__/Button.test.js
@shwinnn
Copy link
Copy Markdown
Contributor Author

shwinnn commented Jan 19, 2021

Fixed!

Copy link
Copy Markdown
Collaborator

@tcbegley tcbegley left a comment

Choose a reason for hiding this comment

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

Thanks @shwinnn!

@tcbegley tcbegley merged commit c852db9 into dbc-team:master Jan 19, 2021
This was referenced Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants