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

Imports css-variables into Persona Bar main styles #6279

Merged
merged 35 commits into from
Jan 4, 2025

Conversation

valadas
Copy link
Contributor

@valadas valadas commented Dec 30, 2024

This replaces hardcoded values in the PersonaBar main styles to use the dnn css-variables which allows matching branding.

  • The previous way to provide a PersonaBar theme still works, so if implementors want their own style to stay unchanged no matter what portal the admin UI is accessed from, that can still work by using actual values instead of the css variables.
  • This does not yet include the common components and the modules themselves, I'll do that in other PRs.

Out of box style (very similar to original)
image

With a different color theme (to match the site under it):
image

This replaces hardcoded values in the PersonaBar main styles to use the dnn css-variables which allows matching branding.

- The previous way to provide a PersonaBar theme still works, so if implementors want their own style to stay unchanged no matter what portal the admin UI is accessed from, that can still work by using actual values instead of the css variables.
- This does not yet include the common components and the modules themselves, I'll do that in other PRs.
@valadas valadas added this to the 10.0.0 milestone Dec 30, 2024
Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

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

@valadas I attempted to start correcting some of these in the browser as suggestions, but it was just bombing out the browser, so maybe you could update the others in your branch and push a new commit now that you see what went wrong?

@valadas
Copy link
Contributor Author

valadas commented Dec 31, 2024

@david-poindexter I ran the file through prettier to bring back all those semi-colons which also made some great enhancements for readability on long rgba funcitons using long variables names, etc. See 3f82fc6 for only this diff, or just let me know what you think with the current state. I also removed 2 usages of color-mix which is a bit flagship for browsers.

@david-poindexter
Copy link
Contributor

@david-poindexter I ran the file through prettier to bring back all those semi-colons which also made some great enhancements for readability on long rgba funcitons using long variables names, etc. See 3f82fc6 for only this diff, or just let me know what you think with the current state. I also removed 2 usages of color-mix which is a bit flagship for browsers.

Oh yeah - this looks MUCH better!

Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

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

Awesome - thanks @valadas 🎉

valadas added a commit to valadas/Dnn.Platform that referenced this pull request Jan 4, 2025
This adds support for css variables to the react common components.
I needed the changes from dnnsoftware#6279 in order to be able to on this. So if it is merged before this one, it becomes easier to review. If not and this one is merged, then this one includes it.
@valadas valadas merged commit 1a5533b into dnnsoftware:release/10.0.0 Jan 4, 2025
3 checks passed
@valadas
Copy link
Contributor Author

valadas commented Jan 4, 2025

Soo, when rebasing I accidently pushed this commit directly to release/10.0.0 by accident.

@dnnsoftware/approvers if there are any concerns with this PR, I can fix that in #6291

Or I can revert the commit if you guys prefer and resubmit this PR.

@david-poindexter
Copy link
Contributor

Soo, when rebasing I accidently pushed this commit directly to release/10.0.0 by accident.

@dnnsoftware/approvers if there are any concerns with this PR, I can fix that in #6291

Or I can revert the commit if you guys prefer and resubmit this PR.

Thanks for the heads up. I think this PR is just fine though and most likely nobody would have issues with it. Let me know if you need my help with anything should someone want this to be reverted.

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.

2 participants