Page MenuHomePhabricator

Force light mode in #sitenotice
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

Follow-up to https://phabricator.wikimedia.org/T360222
Currently the onus is on skins to include the #siteNotice element for full-compatibility with notices - https://codesearch.wmcloud.org/things/?q=id%3D%22siteNotice%22 - we'd like to take this opportunity to define the siteNotice element in MediaWiki core.

User story

As a dark mode user I need the banners/site notices to be readable and accessible always.

Refined Requirement:

Force light mode in the #siteNotice element to ensure banners and site notices are always readable and accessible, regardless of the user's dark mode setting.

BDD

Feature: Force Light Mode in #siteNotice

  Scenario: Ensure light mode for #siteNotice
    Given the user is viewing a Wikipedia page with a site notice
    When the user has dark mode enabled
    Then the #siteNotice element should be displayed in light mode

Test Steps

Test Case 1: Ensure Light Mode for '#siteNotice'

  1. Open the specific URL provided for testing in production: https://ko.m.wikipedia.org/wiki/?minervanightmode=1&banner=wikilovesfolklore2024_banner.
  2. Ensure dark mode is enabled.
  3. AC1: Confirm that the #siteNotice element is displayed in light mode and is readable.

AC

  • We will allow skins to request that MediaWiki core generates the element with id siteNotice.
  • For backwards compatibility this will be an opt-in feature via a skin option.
  • The .notheme class will be added to the element #siteNotice added by core
  • Vector skins will be updated to use the new element
  • Minerva skin will be updated to use the new element
  • add default text color to #siteNotice in resources/src/mediawiki.skinning/interface-site-notice.less [not done: I don't think it's needed.. as notheme sets color?]

QA

Communication criteria - does this need an announcement or discussion?

No

Sign off

  • Open task for updating other skins to use wrapSiteNotice.

Rollback plan

In case of any issues with banners not displaying correctly, it is safe to roll back this change by reverting the skin changes. There is no need to revert the core patch.

e.g. Revert https://gerrit.wikimedia.org/r/1023982 in case of an issue in mobile.

QA Results - Prod

ACStatusDetails
1T361966#9802604

Event Timeline

Copying across from other ticket: We'd need to add the class to both Vector and Minerva skins. Alternatively perhaps Skin::getSiteNotice could contain this logic..?

SToyofuku-WMF subscribed.

@bwang to convert task to use new template

SToyofuku-WMF set the point value for this task to 3.Apr 18 2024, 6:11 PM

@Jdlrobson and @bwang to pair on the task description for the purpose of clarifying the approach (how we will do this in core)

@Jdlrobson I think getSiteNotice could work, but I wanted to point out when I spoke with @Pcoombe I had the impression that the class needed to be on the site notice container, because there are other types of custom content that are sometimes placed there. If that's not a big deal then I'm fine with this approach.

@Pcoombe do you have any thoughts on placing the class through Skin::getSiteNotice
https://phabricator.wikimedia.org/T360222#9690056

I don't know enough about how the skin system works to know if that makes a difference. Which element would that place it on? I think it just needs to be on #siteNotice (which contains MediaWiki-extensions-CentralNotice and/or any sitenotices)

bwang removed bwang as the assignee of this task.Apr 24 2024, 6:34 PM

Change #1023981 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Skin: Handle siteNotice wrapping inside core.

https://gerrit.wikimedia.org/r/1023981

Change #1023982 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Wrap sitenotices in core

https://gerrit.wikimedia.org/r/1023982

Change #1023983 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Wrap sitenotices in core

https://gerrit.wikimedia.org/r/1023983

@Pcoombe there's an opportunity to standardize / formalize the banner container a little more. Currently siteNotice is a DIV created by every individual skin. I've submitted some patches to demonstrate how this would work if you want to try it out.

@Jdlrobson That seems fairly harmless but again: I'm not a skin developer and not well placed to determine if it's useful

Change #1023981 merged by jenkins-bot:

[mediawiki/core@master] Skin: Handle siteNotice wrapping inside core.

https://gerrit.wikimedia.org/r/1023981

Change #1023983 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Wrap sitenotices in core

https://gerrit.wikimedia.org/r/1023983

Change #1023982 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Wrap sitenotices in core

https://gerrit.wikimedia.org/r/1023982

Jdlrobson lowered the priority of this task from High to Medium.
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Backlog to QA on the Web-Team-Backlog (FY2023-24 Q4 Sprint 2) board.
Edtadros updated the task description. (Show Details)
Edtadros subscribed.

Test Result - Prod

Status: ✅ PASS
Environment: Prod
OS: macOS Sonoma
Browser: Chrome
Device: MBA
Emulated Device: NA

Test Artifact(s):

Test Steps

Test Case 1: Ensure Light Mode for '#siteNotice'

  1. Open the specific URL provided for testing in production: https://ko.m.wikipedia.org/wiki/?minervanightmode=1&banner=wikilovesfolklore2024_banner.
  2. Ensure dark mode is enabled.
  3. AC1: Confirm that the '#siteNotice' element is displayed in light mode and is readable.

Everything on the banner/sitenotice is readable and also passes the WCAG contrast checker.

screenshot 351.png (990×1 px, 402 KB)

ovasileva claimed this task.