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

Savi vth forced colors navigation #388

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

savitris
Copy link
Contributor

Navigation bar focus ring in forced-color mode

Improved styling for accordion buttons in forced-color mode

@savitris savitris requested review from bddjong and AliKdhim87 and removed request for bddjong and AliKdhim87 January 23, 2024 11:30
@savitris savitris requested a review from AliKdhim87 January 23, 2024 12:50
@@ -153,6 +153,35 @@ a {
max-width: var(--utrecht-page-content-max-width);
}

@media screen and (-ms-high-contrast: active), screen and (forced-colors: active) {
.utrecht-accordion__header .utrecht-accordion__button {
border: 2px solid white;
Copy link
Member

Choose a reason for hiding this comment

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

whitecurrentColor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gedaan!
P.S. currentColor is nu currentcolor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Robbert
Ik krijg van de Linter dat ik moet currentColor hebben in de code in plaats van currentcolor.

Maar op de MDN site staat er dat nu currentcolor goed is

Mag ik de Linter veranderen zodat het currentcolor goed vindt?

Screenshot 2024-01-29 at 15 21 31

@@ -153,6 +153,35 @@ a {
max-width: var(--utrecht-page-content-max-width);
}

@media screen and (-ms-high-contrast: active), screen and (forced-colors: active) {
.utrecht-accordion__header .utrecht-accordion__button {
Copy link
Member

Choose a reason for hiding this comment

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

Moet de selector echt zo specifiek zijn, of zou .utrecht-accordion__button ook al werken? Geldt ook voor alle andere selectors hier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nee, als ik de eerste class wegdoe, dan gaat het design kaputt.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ik denk dat de accordion style moet worden geüpdate vanuit de designstyle package, zodat ook de VTH dezelfde stijl kan krijgen.
https://github.com/nl-design-system/utrecht/blob/main/components/accordion/css/_mixin.scss

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Robbert
Wat denk jij?

}

@media (prefers-color-scheme: dark) {
.utrecht-accordion__header .utrecht-accordion__button::after {
Copy link
Member

Choose a reason for hiding this comment

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

Misschien kunnen we de icon refactoren naar een <svg> in de HTML, en dan fill="currentColor" gebruiken. Dan hebben we maar 1 SVG nodig en dan kan de CSS veel simpeler blijven.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kun je me aanraden de steps om dat te doen. Ik ben niet zeker waar te beginnen. Hoe kan ik de html code van de accordion modificeren vanuit deze repository?

@savitris savitris requested a review from Robbert March 19, 2024 19:51
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.

3 participants