-
Notifications
You must be signed in to change notification settings - Fork 54
Add support for high contrast mode and print and no css #253
Conversation
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.
Code looks good, could you add screenshots please?
@@ -20,6 +20,7 @@ The `Brand` component is designed to be used where a BBC logo is required as SVG | |||
## Accessibility notes | |||
* Visually hidden text is provided (e.g. for screen reader users) | |||
* `Brand` is keyboard-accessible and provides hover and focus styles | |||
* The brand SVG has support for users with css disabled or high contrast mode enabled. |
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.
👍
@@ -77,11 +85,11 @@ const Brand = ({ brandName }) => ( | |||
focusable="false" | |||
aria-hidden="true" | |||
> | |||
<g fill="#fff" fillRule="evenodd"> | |||
<SvgGroup fill="#000" fillRule="evenodd"> |
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.
Why the change to #000
here?
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.
Its the default for when CSS is disabled
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.
LGTM. Can you please add a screenshots for when CSS isn't disabled (windows only) to confirm the non-high contrast mode behaviour hasn't changed.
@dr3 does this work for setting custom modes (colours) in High Contrast Mode on PC? |
@greenc05 It uses the default font colour that the user assigns |
Fabulous, great work @dr3 |
As we have no means of testing this on a local Psammead environment (excluding Storybook, which wouldn't give an accurate representation as it's an iFrame on a page), we're going to merge this then push to Test. We're going to test the logic on Test. |
Resolves #252
Resolves bbc/simorgh#860
Resolves bbc/simorgh#378
Ensures the svg is always visible if in high contrast mode, when printing and when css is disabled
ORIGINAL
NOW - Windows 7 IE9 Normal
NOW - Windows 7 IE9 High Contrast white
NOW - Windows 7 IE9 High Contrast
NOW - Mac Print
NOW - Mac no css
Known problems that will be fixed in other issues -
Testing notes -
The SVG should be visible (white on red background, or black on white background) when: