Page MenuHomePhabricator

Pre-formatted blocks have no margin
Closed, ResolvedPublic1 Estimated Story Points

Description

Background

In browser-default styling and other skins (e.g. Vector), adjacent <pre> blocks (such as used by SyntaxHighlight) have some spacing between them, like adjacent paragraphs. In Minerva the margin is reset to 0:

User story

As a reader viewing code snippets I'd like to have spacing between blocks.

Requirement

Ensure that pre-formatted blocks have appropriate spacing between them on Minerva skin, similar to the spacing seen on the desktop skin.

BDD

Feature: Spacing Between Pre-formatted Blocks on Minerva Skin

  Scenario: Ensure spacing between pre-formatted blocks
    Given the user is viewing a page with pre-formatted blocks on Minerva skin
    When the page loads
    Then there should be appropriate spacing between pre-formatted blocks
    And the spacing should match that seen on the desktop skin

Test Steps

Test Case 1: Ensure Spacing Between Pre-formatted Blocks

  1. Visit this page on the beta environment using the Minerva skin.
  2. Verify the spacing between pre-formatted blocks.
  3. AC1: Confirm that the spacing matches that seen on the desktop skin.
ObservedExpected
image.png (684×619 px, 95 KB)
image.png (800×534 px, 104 KB)

Acceptance criteria

QA Results - Beta

ACStatusDetails
1T363158#9851742

Event Timeline

Change #1023411 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/skins/MinervaNeue@master] Remove margin reset on <pre> tags

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

Jdlrobson changed the task status from Open to In Progress.Apr 24 2024, 12:06 AM
Jdlrobson removed a project: Regression.

which claimed in the commit message to just move the styles for <pre>, but also introduced a margin reset:

hey @Esanders the margin was already there under: https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/1000021/3/resources/skins.minerva.base.styles/reset.less#b6
So all this change did was increase the strength of the selector from "pre" to ".content pre" so I am having trouble understanding what is broken?

Could you please revise this ticket using the bug template so it's clear what problem you are experiencing is?

Screenshot 2024-04-23 at 2.58.52 PM.png (315×226 px, 22 KB)

Jdlrobson changed the task status from In Progress to Stalled.Apr 24 2024, 12:08 AM
Jdlrobson added a project: Web-Team-Backlog.

Ah, I see this isn't a regression. I've updated the task accordingly. Should be more clear now.

Jdlrobson changed the task status from Stalled to Open.Apr 24 2024, 5:59 PM
Jdlrobson assigned this task to JScherer-WMF.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: JScherer-WMF.

@JScherer-WMF any concerns with mobile matching desktop here?

@JScherer-WMF any concerns with mobile matching desktop here?

None! Go ahead and merge. :-)

Jdlrobson triaged this task as Medium priority.
Jdlrobson lowered the priority of this task from Medium to Low.
Jdlrobson set the point value for this task to 1.

Change #1023411 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Remove margin reset on <pre> tags

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

Edtadros subscribed.

Test Result - Beta

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

Test Artifact(s):

Test Steps

Test Case 1: Ensure Spacing Between Pre-formatted Blocks

  1. Visit this page on the beta environment using the Minerva skin.
  2. Verify the spacing between pre-formatted blocks.
  3. AC1: Confirm that the spacing matches that seen on the desktop skin.

screenshot 99.mov.gif (1×1 px, 454 KB)

Edtadros updated the task description. (Show Details)
Jdlrobson claimed this task.