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

Enable automatic logo insertion #248

Merged
merged 46 commits into from
Jan 16, 2024
Merged

Enable automatic logo insertion #248

merged 46 commits into from
Jan 16, 2024

Conversation

huong-li-nguyen
Copy link
Contributor

@huong-li-nguyen huong-li-nguyen commented Jan 4, 2024

Description

  • Enable automatic logo insertion into the page-header container - this means that the first file found called logo. xxx in the assets folder will be positioned in the left corner
  • The settings container will now move to the right-header container if both dashboard-title and logo are hidden
  • Note: There might always be some custom CSS required to position the logo nicely because a logo can have different paddings, which we cannot predict beforehand. The best thing we can do is to put no restriction on the logo image and always let it take 100% of the height of its parent container. Repositioning can then be done via custom CSS (see code changes in the examples app)

@nadijagraca - thanks for creating the first POC on this! I've had to update it based on the new container structure and logic we will likely introduce here. Depending on your capacity you could help me finish this PR off with docs and tests :)

TO DO in separate PRs:

  • docs + tests

Screenshot

Logo ⛔ + Dashboard Title ✅

no_logo_title

Logo ✅ + Dashboard Title ✅

logo_title

Logo ✅ + Dashboard Title ⛔

logo_no_title

Logo ⛔ + Dashboard Title ⛔

no_logo_no_title

Rectangular Logo ✅ + Dashboard Title ⛔

See this commit to see the relevant CSS changes: d71f101

Screenshot 2024-01-04 at 14 16 56

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 (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 anyone else's intellectual property rights.
    • I have not referenced individuals, products, or companies in any commitments, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

@huong-li-nguyen huong-li-nguyen added the Feature Request 🤓 Issue contains a feature request label Jan 4, 2024
Base automatically changed from dev/dashboard_title_position to main January 16, 2024 13:30
Copy link
Contributor

@nadijagraca nadijagraca left a comment

Choose a reason for hiding this comment

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

LGTM. I just have one small comment on css.

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!

vizro-core/src/vizro/models/_dashboard.py Show resolved Hide resolved
@huong-li-nguyen huong-li-nguyen enabled auto-merge (squash) January 16, 2024 17:35
@huong-li-nguyen huong-li-nguyen merged commit 20b1d3f into main Jan 16, 2024
33 checks passed
@huong-li-nguyen huong-li-nguyen deleted the feat/enable_logo branch January 16, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request 🤓 Issue contains a feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants