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

Brand can't be seen in FireFox when a user changes the colour preferences #736

Closed
pjlee11 opened this issue Jul 1, 2019 · 5 comments · Fixed by #787
Closed

Brand can't be seen in FireFox when a user changes the colour preferences #736

pjlee11 opened this issue Jul 1, 2019 · 5 comments · Fixed by #787
Assignees
Labels
articles-av-epic bug Something isn't working high priority ws-articles Tasks for the WS Articles Team

Comments

@pjlee11
Copy link
Contributor

pjlee11 commented Jul 1, 2019

Describe the bug
When a user changes the colour preferences in FireFox the brand SVG is displayed as white on a white background (if a white background colour has been chosen) and therefore can't be seen.

To Reproduce
Steps to reproduce the behaviour:

  1. Go to FireFox
  2. Click on Firefox -> Preferences -> Languages & Appearance -> Colours...
  3. For Override the colours... select Always and then click OK
  4. Visit https://www.bbc.co.uk/news/articles/c8xxl4l3dzeo
  5. See error as Brand isn't visible

Expected behaviour
I should still be able to see the brand SVG when:

  1. User has changed colour preferences in Firefox (including with a white background)
  2. User has no CSS
  3. User has selected a High Contrast mode on PC with a white background and also a custom mode (Point 4 on https://bbc-news.github.io/accessibility-news-and-you/accessibility-news-and-testers)
  4. Low priority but ideally you should still be able to see the logo when printing an article - We already have some CSS for this - which should be unaffected if applying the recommended fix in the comment below https://github.com/bbc/psammead/blob/latest/packages/components/psammead-brand/src/index.jsx#L72

Screenshots
image

Additional context
As this bug is due to FireFox override the CSS styles we have defined for the page there might not be an easy fix. Ideally we would need something on the page to indicate that the styles have been overridden.

@pjlee11 pjlee11 added bug Something isn't working ws-home Tasks for the WS Home Team ws-articles Tasks for the WS Articles Team simorgh-core-stream labels Jul 1, 2019
@greenc05
Copy link
Contributor

greenc05 commented Jul 1, 2019

We could try applying fill: currentColor instead of fill: rgb(255, 255, 255) to the SVG to fix this. May also need to apply a colour of white to the wrapping link... Remember to ensure this can also still be seen in High contrast mode on PC, as well as with CSS turned off.

@greenc05 greenc05 changed the title Brand can't be seen in FireFox high contrast mode Brand can't be seen in FireFox when a user changes the colour preferences Jul 1, 2019
@dr3 dr3 removed the ws-home Tasks for the WS Home Team label Jul 1, 2019
@greenc05 greenc05 added shared-components and removed ws-articles Tasks for the WS Articles Team labels Jul 2, 2019
@jamesdonoh jamesdonoh added ws-articles Tasks for the WS Articles Team and removed shared-components simorgh-core-stream labels Jul 2, 2019
@jamesbhobbs
Copy link
Contributor

@greenc05 you're not in today to discuss, my view.... I think this is probably a P2 priority issue (where P1 is highest for anyone unfamiliar with our grading system). It's clearly P1 when it occurs, but it only occurs when colour-preferences are changed. Nevertheless P2 means we need to fix urgently, so we will get someone to look at it soon.

PS I believe this would also affect WS front pages in simorgh which will go live soon with a far larger audience.

@dr3
Copy link
Contributor

dr3 commented Jul 4, 2019

@benjaminhobbs It does cover WS (soz i removed the label to avoid confusion with high priority label) @greenc05 Agreed that is isn't an immediate issue for frontpages due to our inclusion of the Nav

@j-pendlebury
Copy link
Contributor

Could this potentially be fixed by #704?

Currently psammead-brand is using an old version of psammead-assets, which does not contain the changes made in PR above.

@pjlee11
Copy link
Contributor Author

pjlee11 commented Jul 5, 2019

@jblaut Potentially, definitely worth investigating

@12 12 self-assigned this Jul 5, 2019
@12 12 closed this as completed in #787 Jul 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
articles-av-epic bug Something isn't working high priority ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants