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

WHCM styles improvements #455

Merged
merged 3 commits into from
Feb 6, 2024
Merged

WHCM styles improvements #455

merged 3 commits into from
Feb 6, 2024

Conversation

shyusu4
Copy link
Member

@shyusu4 shyusu4 commented Jan 29, 2024

#454

Description

This pull request addresses styling issues related to windows high contrast mode. The following issues have been fixed:

  • CTA button:
    • Arrow icon isn't visible when hovered.
    • Inconsistent transition speeds between the title and subtitle.
  • Header navigation: Arrow icon within CTA link in sub-navigation panel isn’t visible.
  • Code snippet block: Code blocks should have a clear separation from regular content.
  • Footer:
    • All social media, “sponsor Wagtail”, and “contribute code” icons aren't visible.
    • Social media icons are not using the correct color (link text).

Note - Dark/light mode theme toggle was meant to not be displayed in HCM but used prefers-contrast instead of forced-colors active.

Screenshots
Screenshot (65) Screenshot (63) Screenshot (64)
Screenshot (68) Screenshot (69) Screenshot (70)
Screenshot (72) Screenshot (73) Screenshot (74)
Screenshot (77) Screenshot (75) Screenshot (76)

@shyusu4 shyusu4 marked this pull request as ready for review January 31, 2024 10:13
stroke: $color--white;
stroke-width: 1px;
stroke: currentColor;
stroke-width: 4px;

Choose a reason for hiding this comment

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

Here we're affecting designs in a non-contrast mode, we need to add media query to target forced-colors only

Choose a reason for hiding this comment

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

The stroke: currentColor does not seem to have any effect on the regular rendering. In my opinion the stroke-width increase would be an improvement to the base design, it's not my place to decide that, so I agree with Albina's suggestion to move these rules into a media query.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @Scotchester @albinazs - I've moved the use of currentColor into a media query, but left stroke-width because this is part of another minor issue in the audit. I think it would be better to keep the increased width so that the nav CTA is more distinguishable as a link.

Choose a reason for hiding this comment

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

Ah, gotcha! If we are considering the thin stroke an accessibility issue as well, then I am happy to have the change :)

@albinazs
Copy link

thank you, @shyusu4! great stuff!:)

@Scotchester
Copy link

Good stuff, @shyusu4! Just the one thing where we need the media query that Albina noticed before we can approve.

@thibaudcolas thibaudcolas merged commit 163715e into wagtail:main Feb 6, 2024
3 checks passed
@thibaudcolas
Copy link
Member

🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants