-
Notifications
You must be signed in to change notification settings - Fork 433
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
Minor css variables fixes for header & navbar #2441
Minor css variables fixes for header & navbar #2441
Conversation
Thanks @alexandrevryghem. You are on a roll with the Angular fixes. I use a custom theme based on the Before this PR: After this PR: It's a minor issue, but I can't override the dropdown menu background without theming the diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss
index 65de77b60..1bc80d32c 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss
@@ -6,6 +6,7 @@
}
.dropdown-menu {
+ background-color: var(--ds-navbar-bg);
overflow: hidden;
min-width: 100%;
border-top-left-radius: 0; So I'm not sure about this PR. I don't see the benefit in my case! 😛 |
@alanorth This is weird, because the expandable navbar section uses the same color as the navbar links, so they should both use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks @alexandrevryghem ! Tested this and verified that header/navbar styles can now be modified via variables using either the dspace
theme or custom
theme. Default styling remains the same as well.
Successfully created backport PR for |
Description
When you create a custom theme who doesn't extend the
dspace
theme, you can't change the background color of the header. It is also not possible to change the navbar background color using CSS variables.Instructions for Reviewers
List of changes in this PR:
HeaderComponent
insrc/app
now also sets thebackground-color: var(--ds-header-bg)
--ds-navbar-bg
(this is maybe not useful if you use the navbar in the same way as thedspace
theme. But for custom themes who use theHeaderComponent
&NavbarComponent
fromsrc/app
this is useful, because the the navbar is rendered underneath the header)NavbarComponent
incorrectly dividing the css variable--ds-content-spacing
by 3Include guidance for how to test or review your PR.
themes
config in yourconfig.yml
file entirely and don't provide any themes:--ds-header-bg
&--ds-navbar-bg
insrc/styles/_custom_variables.scss
and check that they are correct in the UIconfig.yml
file to use thedspace
theme:--ds-header-bg
&--ds-navbar-bg
insrc/themes/dspace/styles/_theme_css_variable_overrides.scss
and check that they are correct in the UIChecklist
yarn lint
yarn check-circ-deps
)package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.