search

visitscotland / vs-component-library

VisitScotland Component Library
Other
0 stars 0 forks source link

CSS classes are duplicated within a component #300

Open markup-mitchell opened 8 months ago

markup-mitchell commented 8 months ago

Raised by @markup-mitchell

Describe the bug

A clear and concise description of what the bug is.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://www.visitscotland.com/places-to-go/edinburgh/things-to-do/7-day-itinerary
  2. Open devtools
  3. Search the inspector for .row
  4. Jump through matches until you see classnames repeated within some elements

Expected behavior

Since a repeated class could re-instate properties intended to be overriden, any unique classname should only appear once inside the class parameter.

Page URL

Currently looking at an itinerary, but have noticed this across the project. https://www.visitscotland.com/places-to-go/edinburgh/things-to-do/7-day-itinerary

Environment

Seems to be everywhere: prod, storybook, chromatic builds...

Browser

Chrome, Firefox.. it's in the markup

Device and version

Dell laptop

Impact on end user

None perhaps?

Impact on product development

This is more of a smell than a behavioural defect; is a build step being repeated and generating extraneous classes?

markup-mitchell commented 8 months ago

Real-world example.

Chromatic is flagging a UI bug — the component is shorter than the baseline. This seems to be due to the "Time to explore:" microcopy element, which has no margin-bottom in the new version.

The classList in both branches is identical:

<div class="row vs-description-list list-inline my-4 mb-0 justify-content-start vs-description-list list-inline my-4 mb-0 justify-content-start" tag="dl">

mb-0 is respected in the new version but overridden in the baseline. It's probably best not to have utility classes overriding each other within a single element, but if it does happen the results should be predictable. IMO the new branch is better because mb-0 should trump my-4 in terms of specificity, though I don't know whether this is a deliberate effect of BS5 re-arranging the order in which the utils are processed or just a coincidence in this case.

In any event the duplication of utils really confuses matters because it not only makes the outcome less predictable, it also obscures the intent.

UPDATE: Going to leave this comment here but hide it as it's not necessarily relevant to the issue. The UI bug is resolved by removing one of the utils from the storybook story, so the root cause probably is best attributed to the authoring of the pattern as opposed to problem at the component-level. Presumably now the element will look like <div class="row vs-description-list list-inline my-4 justify-content-start vs-description-list list-inline my-4 justify-content-start" tag="dl"> — the utils still repeated but without mb-0. The duplication issue remains.