Page MenuHomePhabricator
Create Task
Maniphest T205341

Split up reset.css rules to specific element selectors only where needed and remove file altogether
Open, NormalPublic
Actions

Description

MinvervaNeue has been using for historic reasons[1] a fork of Erik Meyer's reset.
This has become seen as disadvantegous for a few reasons in contrast to normalize.css:

  1. normalize.css preserves useful defaults rather than "unstyling" everything.
  2. It corrects some common bugs that are out of scope for reset.css.
  3. It doesn't clutter your dev tools.
  4. It is more modular and can be broken down to the elements MinveraNeue wants to support

On top of normalize, we could also add some opinionated styles inspired by collections like sanitize.css and clearly mark them so.

[1]: Invented first to tame T42751 6 years ago.

Proposed change

Subtasks (and patches by @Volker_E; all of which are small) provide the split of the monster selector in more modular code. In a feature branch:

  • Patches which split up code only
  • Patches which add additional properties, inspired by normalize.css

Reading web will

  • review and merge these all together and
  • QA the changes in one go using this task

Acceptance criteria

  • Unnecessary reset rules are removed (T217631) including for headings (T217626) and audio/video (T217629). Note video tag is not currently used in mobile site so risk here is negligible.
  • Table styles have been moved to a separate file tables.less and ordered by specificity (tracked in subtask T217620)
  • Image styles are moved to images.less. Border reset is moved to links.less (tracked in subtask T217630)
  • Form styles are moved out of the reset to more logical places (T217622)

QA steps

We are looking for visual regressions and should test on Firefox, iOS and Chrome.

We'll look at a variety of articles and compare production versions vs the versions on reading web staging. The suggested articles are:

Pay particular attention to the following:

  • Tables and infoboxes look the same no visual regressions
  • Images look the same, no regressions.
  • Any font sizes that are different
  • Any margins that are different
  • Headings look same
  • Thumbnails and audios look the same

In addition to this check for visual regression on the forms on the following special pages:

  • Special:Watchlist
  • Special:Userlogin
  • Special:RecentChanges
  • Special:Contributions
  • Special:Search

and overlays:

  • talk #/talk/new
  • editor #/editor/0
  • search #/search

Sign off steps

  • Make sure all subtasks are merged.

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson triaged this task as Normal priority.EditedSep 24 2018, 9:31 PM

getting rid of this monster selector https://gerrit.wikimedia.org/g/mediawiki/skins/MinervaNeue/+/refs/changes/59/462559/2/resources/skins.minerva.base.reset/reset.less#7 in respect to #3 not cluttering dev tools would make a lot of sense.

I'm open to it, just not a top priority right now. I've added it on the list of things to talk about with the team.

gerritbot added a subscriber: gerritbot.Mar 5 2019, 2:10 AM

Change 494400 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Remove ol overrides to ensure list styles in non-Arabic number scripts

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

gerritbot added a comment.Mar 5 2019, 2:38 AM

Change 494402 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move (opinionated) table styles to tables stylesheet

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

Volker_E updated the task description. (Show Details)Mar 5 2019, 2:52 AM
Volker_E added a subscriber: matmarex.
gerritbot added a comment.Mar 5 2019, 3:30 AM

Change 494405 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Remove unnecessary table font-size reset

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

gerritbot added a comment.Mar 5 2019, 3:44 AM

Change 494406 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move html and body styles to ui.less

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

gerritbot added a comment.Mar 5 2019, 6:51 AM

Change 494411 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move form specific styles to new 'forms.less' file

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

gerritbot added a comment.Mar 5 2019, 7:02 AM

Change 494412 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move headings specific reset to 'headings.less'

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

gerritbot added a comment.Mar 5 2019, 7:14 AM

Change 494414 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move p, pre and blockquote styles to 'text.less'

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

gerritbot added a comment.Mar 5 2019, 7:26 AM

Change 494416 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move sensitive audio & video default styles out of 'reset.less'

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

gerritbot added a comment.Mar 5 2019, 7:38 AM

Change 494417 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move sensitive img reset rules to specific LESS files

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

gerritbot added a comment.Mar 5 2019, 7:49 AM

Change 494418 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Remove unnecessary phrasing content reset and handle address

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

Jdlrobson renamed this task from MinervaNeue should make use of normalize CSS instead of reset to [EPIC] MinervaNeue should make use of normalize CSS instead of reset.Mar 5 2019, 9:59 PM
Jdlrobson moved this task from Tracking to Epics/Goals on the Readers-Web-Backlog board.
Jdlrobson edited projects, added Readers-Web-Backlog; removed Readers-Web-Backlog (Tracking).
Jdlrobson added a project: Epic.
gerritbot added a comment.Mar 6 2019, 11:20 PM

Change 494406 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Move html and body styles to 'ui.less'

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

Jdlrobson renamed this task from [EPIC] MinervaNeue should make use of normalize CSS instead of reset to MinervaNeue should make use of normalize CSS instead of reset.Mar 8 2019, 12:36 AM
Jdlrobson removed a project: Epic.
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Mar 8 2019, 12:39 AM
Jdlrobson moved this task to Upcoming on the Readers-Web-Backlog board.

Adding to upcoming for estimation. Volker will need our support in QA and code review.

gerritbot added a comment.Mar 11 2019, 3:49 AM

Change 495628 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/MobileFrontend@master] Add sensitive form element defaults

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

ovasileva set the point value for this task to 8.Mar 12 2019, 5:57 PM
Jdlrobson moved this task from Upcoming to Triaged but Future on the Readers-Web-Backlog board.EditedMar 12 2019, 5:59 PM
Jdlrobson added a subscriber: ovasileva.

We talked about this in grooming today. The team was concerned at the risk associated with all these changes as we don't really have adequate ways to test these changes and these changes also impact apps (recently we broke their dark mode with similar changes). Even though you've broke them down into subtasks, we'd want to do them in one go.

We do think these changes are good and help make our CSS easier to maintain on the long term but we don't feel comfortable committing to merging and QAing these changes right now. Please talk to @ovasileva about getting this work scheduled.

Jdlrobson added a subscriber: alexhollender.Mar 25 2019, 6:04 PM

@Jdrewniak @Volker_E @alexhollender and I met and agreed:

  1. rename the task to explain we're actually removing CSS not adopting normalize.css
  2. We'll review and merge all the patches to a feature branch
  3. We'll prepare a QA task and patch to merge this code to master, put it on staging for QA by Alex and Edward
  4. When Edward+Alex give the green light we'll merge it to master (all the above reduces the risk from our end so this becomes a small task rather than a Large / 8)
Volker_E renamed this task from MinervaNeue should make use of normalize CSS instead of reset to Split up reset.css rules to specific element selectors only where needed and remove file altogether.Mar 25 2019, 7:07 PM
Volker_E updated the task description. (Show Details)
gerritbot added a comment.Apr 4 2019, 4:52 PM

Change 495628 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/extensions/MobileFrontend@master] Add sensitive form element defaults

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

gerritbot added a comment.Apr 4 2019, 4:58 PM

Change 494411 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove form styles from 'reset.less'

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

gerritbot added a comment.Apr 4 2019, 4:58 PM

Change 494412 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Move headings specific reset to 'headings.less'

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

gerritbot added a comment.Apr 4 2019, 5:08 PM

Change 495628 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Add sensitive form element defaults

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

gerritbot added a comment.Apr 4 2019, 6:33 PM

Change 494411 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove form styles from 'reset.less'

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

gerritbot added a comment.Apr 4 2019, 9:30 PM

Change 494416 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Move sensitive audio & video default styles out of 'reset.less'

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

gerritbot added a comment.Apr 4 2019, 9:31 PM

Change 494417 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Move sensitive img reset rules to specific LESS files

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

gerritbot added a comment.Apr 4 2019, 9:35 PM

Change 494405 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove unnecessary table font-size reset

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

gerritbot added a comment.Apr 4 2019, 9:49 PM

Change 494416 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Move sensitive audio & video default styles out of 'reset.less'

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

gerritbot added a comment.Apr 4 2019, 9:49 PM

Change 494417 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Move sensitive img reset rules to specific LESS files

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

gerritbot added a comment.Apr 4 2019, 11:56 PM

Change 494412 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Move headings specific reset to 'headings.less'

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

gerritbot added a comment.Apr 5 2019, 10:37 PM

Change 494418 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove unnecessary phrasing content reset and handle address

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

gerritbot added a comment.Apr 11 2019, 11:56 PM

Change 494402 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Move (opinionated) table styles to tables stylesheet

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

Jdlrobson added a comment.Apr 24 2019, 2:40 AM
This comment was removed by Jdlrobson.
gerritbot added a comment.Apr 24 2019, 2:49 AM

Change 494402 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Move (opinionated) table styles to tables stylesheet

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

gerritbot added a comment.Apr 24 2019, 2:49 AM

Change 494405 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove unnecessary table font-size reset

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

gerritbot added a comment.Apr 24 2019, 2:50 AM

Change 494418 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove unnecessary phrasing content reset and handle address

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

Jdlrobson added a comment.Apr 24 2019, 10:31 AM

I've merged the existing patches, and rebased the feature branch on master.
I'm seeing several problems with the existing code relating to buttons and inputs that are rendered with JavaScript. My guess is these styles are only being loaded on special pages (which is good) but their CSS should also be loaded in JS:

Download button has border and is miniscule:

Buttons:

Close icon:

Search input not correct size:

Close icon:

gerritbot added a comment.Apr 29 2019, 7:53 PM

Change 494400 had a related patch set uploaded (by Bartosz Dziewoński; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Remove ol overrides to ensure list styles in non-Arabic number scripts

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

gerritbot added a comment.Apr 29 2019, 8:05 PM

Change 494400 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove ol overrides to ensure list styles in non-Arabic number scripts

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

Jdlrobson added a comment.May 1 2019, 7:17 PM

Progress is blocked on https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/494414/ and fixes to https://phabricator.wikimedia.org/T205341#5134218
Once that's amended, reviewed and merged, I'll prepare a patch to apply these changes to the master branch and schedule QA.

gerritbot added a comment.May 1 2019, 7:44 PM

Change 494414 had a related patch set uploaded (by Jforrester; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move p, pre and blockquote styles to 'text.less'

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

gerritbot added a comment.May 6 2019, 10:16 PM

Change 508436 had a related patch set uploaded (by Jdlrobson; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Move p, pre and blockquote styles to 'text.less'

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

gerritbot added a comment.May 6 2019, 10:17 PM

Change 508436 abandoned by Jdlrobson:
Move p, pre and blockquote styles to 'text.less'

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

gerritbot added a comment.May 6 2019, 10:18 PM

Change 508436 restored by Jdlrobson:
Move p, pre and blockquote styles to 'text.less'

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

gerritbot added a comment.May 6 2019, 10:18 PM

Change 508436 abandoned by Jdlrobson:
Move p, pre and blockquote styles to 'text.less'

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

gerritbot added a comment.May 7 2019, 6:40 PM

Change 494414 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Address p, pre and blockquote styles in 'text.less'

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

Jdlrobson removed a project: Patch-For-Review.May 7 2019, 6:45 PM

The comments in T205341#5134218 still stand and will need to be resolved before we can prepare the patch for QA/ merge to master.

gerritbot added a comment.May 7 2019, 6:47 PM

Change 508675 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Merge remote-tracking branch 'origin/reset-cleanup' into master

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

Volker_E added a comment.May 7 2019, 9:37 PM

@Jdlrobson So the scope of the task was originally defined by the tasks written then.
Nonetheless I would like to extend it by moving the remaining bits out of reset.less and getting rid of it completely.
That's my personal success criterium for this task.

Jdlrobson added a comment.May 7 2019, 11:02 PM

Im not following. The current branch causes UI regressions - small icons and badly styled buttons. These are caused by the reset cleanup that has been merged so far so will have to be fixed before we merge to master and QA. Im keen to merge this too and share your goals but I can't do that while these regressions exist.

gerritbot added a comment.May 8 2019, 9:41 PM

Change 508946 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/MobileFrontend@master] Use font-size: 100% instead of inherit to ensure form element sizing

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

gerritbot added a comment.May 8 2019, 10:10 PM

Change 508950 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/MobileFrontend@master] Remove browser default padding on button and input

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

gerritbot added a comment.May 9 2019, 12:12 PM

Change 509045 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove unnecessary line-height definitions

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

gerritbot added a comment.May 9 2019, 1:01 PM

Change 509045 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove unnecessary line-height definitions

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

gerritbot added a comment.May 9 2019, 1:23 PM

Change 509054 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Move list specific styles to lists.less

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

gerritbot added a comment.May 9 2019, 2:07 PM

Change 509062 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Introduce 'forms.less' with styling most common UI form elements

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

gerritbot added a comment.May 9 2019, 2:15 PM

Change 509064 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove emptied 'reset.less' and its skin dependency

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

Volker_E added a comment.May 9 2019, 2:16 PM

@Jdlrobson I couldn't reproduce the button background and border issues. With the all the additional patches above, there's only one thing remaining from my POV:
When focussing the overlay the search box moves further than before. On a first skim, I couldn't pin-point what rule causes this.

gerritbot added a comment.May 9 2019, 5:14 PM

Change 509115 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@reset-cleanup] Fix UI regressions with icons, inputs and p elements

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

gerritbot added a comment.May 9 2019, 5:24 PM

Change 509054 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Move list specific styles to lists.less

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

gerritbot added a comment.May 9 2019, 5:59 PM

Change 509062 abandoned by VolkerE:
Introduce 'forms.less' with styling most common UI form elements

Reason:
Abandoned for I68551aa0969

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

gerritbot added a comment.May 9 2019, 9:24 PM

Change 509115 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Fix UI regressions with icons, inputs and p elements

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

gerritbot added a comment.May 9 2019, 10:00 PM

Change 509064 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Remove emptied 'reset.less' and its skin dependency

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

Jdlrobson added a comment.May 10 2019, 2:21 AM

Doh, I missed two things :/



https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/509178 Additional icon resets for button elements fixes those.

Also problem with overlay buttons and header height


This needs a skinStyle inside skinStyles/mobile.startup/Overlay.less (haven't got time to check that now)

Once those are fixed do

git fetch origin
git checkout origin/master
git merge --no-ff  origin/reset-cleanup
Amend the commit with the change ID I66597b481ca6077105de7ebdbbb504a2f3116ccc
gerritbot added a comment.May 10 2019, 7:40 AM

Change 509178 had a related patch set uploaded (by VolkerE; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@reset-cleanup] Additional icon resets for button elements

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

gerritbot added a comment.May 10 2019, 9:57 AM

Change 509178 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Additional icon resets for button elements

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

gerritbot added a comment.May 10 2019, 2:51 PM

Change 509425 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@reset-cleanup] Fix button size in .header-actions

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

gerritbot added a comment.May 13 2019, 5:28 PM

Change 509425 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@reset-cleanup] Fix button size in .header-actions

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

Volker_E updated the task description. (Show Details)May 14 2019, 3:38 PM
gerritbot added a comment.Jun 4 2019, 12:43 PM

Change 508946 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Use font-size: 100% instead of inherit to ensure form element sizing

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

Jdlrobson removed the point value for this task.Mon, Jul 8, 11:40 PM

@Jdrewniak @Volker_E what's left here? I don't see any open patchsets and unless I'm mistaken https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/508675/-1..4 did not land.

I've rebased it, so the question now is do we need to schedule some QA and estimation?
Story points seem irrelevant at this point.
I have a sync with Olga on Wednesday so can arrange some time if I know what's needed to push this over the finish line.

Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL