Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Add support for high contrast mode and print and no css #253

Merged
merged 7 commits into from
Jan 8, 2019

Conversation

dr3
Copy link
Contributor

@dr3 dr3 commented Jan 3, 2019

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

  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

ORIGINAL

image

NOW - Windows 7 IE9 Normal

image

NOW - Windows 7 IE9 High Contrast white

image

NOW - Windows 7 IE9 High Contrast

image

NOW - Mac Print

image

NOW - Mac no css

image

Known problems that will be fixed in other issues -

  • Gel grid not working on IE
  • SVG is full width when css is disabled

Testing notes -
The SVG should be visible (white on red background, or black on white background) when:

  • In high contrast modes in IE
  • When printing the articles
  • When CSS is disabled

@dr3 dr3 added the alpha-3 label Jan 3, 2019
@dr3 dr3 self-assigned this Jan 3, 2019
Copy link
Contributor

@ChrisBAshton ChrisBAshton left a 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.
Copy link
Contributor

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">
Copy link
Contributor

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?

Copy link
Contributor Author

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

radiocontrolled
radiocontrolled previously approved these changes Jan 3, 2019
Copy link
Contributor

@radiocontrolled radiocontrolled left a 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 dr3 mentioned this pull request Jan 3, 2019
1 task
bcmn
bcmn previously approved these changes Jan 3, 2019
@dr3 dr3 dismissed stale reviews from bcmn and radiocontrolled via b7eb00f January 3, 2019 17:21
@greenc05
Copy link
Contributor

greenc05 commented Jan 7, 2019

@dr3 does this work for setting custom modes (colours) in High Contrast Mode on PC?

@dr3
Copy link
Contributor Author

dr3 commented Jan 7, 2019

@greenc05 It uses the default font colour that the user assigns
image

@greenc05
Copy link
Contributor

greenc05 commented Jan 7, 2019

Fabulous, great work @dr3

@jamesbrumpton
Copy link
Contributor

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.

@dr3 dr3 merged commit 85f3bd6 into latest Jan 8, 2019
@dr3 dr3 deleted the WIN-IE-HIGH-CONTRAST branch January 8, 2019 11:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
7 participants