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

[Feat] Create vizro.figures.library to enable usage in pure Dash app #578

Merged
merged 67 commits into from
Aug 19, 2024

Conversation

huong-li-nguyen
Copy link
Contributor

@huong-li-nguyen huong-li-nguyen commented Jul 10, 2024

Description

This is the first PR to create a library for our components to be used in a standalone Dash app, independent of the Vizro framework. For context: The key performance indicator (KPI) cards are the first components eligible for use in a pure Dash app. In the future, we might add custom charts and other components as well. This PR lays the foundation for enabling these additions.

To ensure KPI components, and future components, work seamlessly outside of Vizro, we made the following adjustments:

  • Created vizro.figures.library to house KPI card Dash components usable outside the Vizro framework.
  • Enabled offline use of the Google Material Icons library.
  • Shipped CSS and fonts with the component library. Without this, KPI cards will appear unstyled and fail to render Google Material Icons correctly. Vizro KPI cards used in a standalone Dash app will not look the same as within Vizro but will adopt the look of the provided Bootstrap theme.
  • Updated the Vizro-Bootstrap theme and modified the KPI Cards to look acceptable across all Bootstrap themes (https://github.com/McK-Private/vizro-bootstrap/pull/30)

Detailed code comments from @antonymilne explain these changes further (see changed files)

Screenshot

Vizro KPI Cards inside Vizro or with vizro-bootstrap (not public yet)
Screenshot 2024-08-16 at 11 47 10

Vizro KPI Cards in pure dash app with other bootstrap themes:

dbc.themes.BOOTSTRAP
Screenshot 2024-08-16 at 11 50 01

dbc.themes.SANDSTONE
Screenshot 2024-08-16 at 11 48 22

dbc.themes.CYBORG
Screenshot 2024-08-16 at 11 49 10

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
    • I have not referenced individuals, products or companies in any commits, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

@pre-commit-ci pre-commit-ci bot temporarily deployed to circleci_secrets July 10, 2024 11:15 Inactive
@pre-commit-ci pre-commit-ci bot temporarily deployed to circleci_secrets July 10, 2024 11:21 Inactive
@antonymilne
Copy link
Contributor

Potential candidates for the naming of the module:

undecorated
unwrapped
unaltered
raw
plain
pure
Open for any other suggestion 😄 I am either for undecorated or unwrapped.

I like unwrapped or raw best.

@huong-li-nguyen huong-li-nguyen self-assigned this Jul 15, 2024
@pre-commit-ci pre-commit-ci bot temporarily deployed to circleci_secrets July 15, 2024 08:57 Inactive
@pre-commit-ci pre-commit-ci bot temporarily deployed to circleci_secrets July 22, 2024 08:39 Inactive
@antonymilne antonymilne marked this pull request as ready for review August 15, 2024 12:05
@huong-li-nguyen huong-li-nguyen changed the title [Tidy] Add undecorated module for KPI cards [Feat] Create vizro.figures.library to enable usage of components in pure Dash app Aug 16, 2024
}

.card-kpi .card-body {
align-items: center;
color: var(--text-secondary);
color: var(--bs-secondary);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important for future reference: In case we do add new components, colors need to be taken from the bootstrap variables, otherwise the new components are not compatible with other bootstrap themes.

Copy link
Contributor

@maxschulz-COL maxschulz-COL Aug 19, 2024

Choose a reason for hiding this comment

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

Shall we document this in our wiki, and potentially also in our code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a ticket on creating a guide on an "intro to css" where we could add this in 👍

So i wouldn't add it to this PR for now. All PRs changing CSS would currently go through @nadijagraca, @pruthvip15 and/or me anyway :)

@huong-li-nguyen huong-li-nguyen changed the title [Feat] Create vizro.figures.library to enable usage of components in pure Dash app [Feat] Create vizro.figures.library to enable usage in pure Dash app Aug 16, 2024
@huong-li-nguyen
Copy link
Contributor Author

huong-li-nguyen commented Aug 16, 2024

  • manual testing with packaged vizro

@antonymilne - works perfectly expected! 🥳 🍾 💯 🚀

1. Vizro app with serve_locally=True and NO internet ✅
Screenshot 2024-08-16 at 12 24 39

2. Vizro app with serve_locally=False and internet ✅
Screenshot 2024-08-16 at 12 23 08

3. Pure dash app with serve_locally=True, non-vizro bootstrap theme and NO internet ✅
Screenshot 2024-08-16 at 12 13 06

4. Pure dash app with serve_locally=False, non-vizro bootstrap theme and internet ✅
Screenshot 2024-08-16 at 12 16 06

Copy link
Contributor

@petar-qb petar-qb left a comment

Choose a reason for hiding this comment

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

Wonderful work!

I'm curious: How far are we and do we actually want to support applying any bootstrap theme inside a Vizro application just by propagating the theme to the Vizro external_stylesheets argument?

vizro-core/examples/scratch_dev/pure-app.py Outdated Show resolved Hide resolved
vizro-core/src/vizro/__init__.py Show resolved Hide resolved
vizro-core/src/vizro/static/css/vizro-bootstrap.min.css Outdated Show resolved Hide resolved
@huong-li-nguyen
Copy link
Contributor Author

huong-li-nguyen commented Aug 19, 2024

I'm curious: How far are we and do we actually want to support applying any bootstrap theme inside a Vizro application just by propagating the theme to the Vizro external_stylesheets argument?

It's definitely on our roadmap - there is a detailed plan written out here and we've started development a while ago if you remember. You can track the progress there as well. However, we've paused it for a while given other priorities.

The main goal is to create a complete Vizro Bootstrap theme and make it open-source so that anyone using a pure Dash app can use it too. We want all our CSS to be part of this theme, with only a few tweaks for third-party libraries like Dash-Mantine and AG-Grid. Everything else should be converted to Bootstrap. This way, we have one consistent source, making it easier to add other custom components and it will be another offering of Vizro :)

It was always planned that people should be able to change the "skin" of the dashboard easily. The Bootstrap theme is the most straightforward way of doing this without having lots of overwrites 👍

Copy link
Contributor

@maxschulz-COL maxschulz-COL left a comment

Choose a reason for hiding this comment

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

😍 Wow this is super cool! Tried out a few bootstrap themes, and it's really cool to see how the cards can be used in a pure dash app, and how they work well with those themes. Really great work @antonymilne @huong-li-nguyen !! ⭐

I have almost no comments - let me know if I should test something more manual, but going through the scratch app, all seemed fine 💪

vizro-core/docs/pages/API-reference/figure-callables.md Outdated Show resolved Hide resolved
}

.card-kpi .card-body {
align-items: center;
color: var(--text-secondary);
color: var(--bs-secondary);
Copy link
Contributor

@maxschulz-COL maxschulz-COL Aug 19, 2024

Choose a reason for hiding this comment

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

Shall we document this in our wiki, and potentially also in our code?

vizro-core/src/vizro/__init__.py Show resolved Hide resolved
Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for all your patience and perseverance seeing this through!

vizro-core/src/vizro/__init__.py Outdated Show resolved Hide resolved
vizro-core/src/vizro/__init__.py Show resolved Hide resolved
@huong-li-nguyen huong-li-nguyen enabled auto-merge (squash) August 19, 2024 09:56
@huong-li-nguyen huong-li-nguyen merged commit b32da0d into main Aug 19, 2024
32 checks passed
@huong-li-nguyen huong-li-nguyen deleted the tidy/create-module-pure-functions branch August 19, 2024 10:04
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.

4 participants