Page MenuHomePhabricator

[SPIKE] What's going on with BEM in Vector?
Closed, ResolvedPublic0 Estimated Story Points

Description

BEM has been proposed multiple times in the codebase. For now we appear to be using a modified version of BEM using '-' and it's not clear why and what the benefits are.

Goals

  • Decide whether we plan to ever adopt BEM and if so what our strategy for getting there is
  • Agree on how to name classes in Vector currently and document in the codebase
  • Review existing names
  • Ensure the storybook reflects the agreed names of components that are used by designers and engineers.

(See T253671#7261552 for details)

If applicable

Are we following a BEM(-like) terminology in components? Example: Current 'EmphasizedSidebarAction.less' might be better expressed as 'ActionSidebarEmphasized' or 'LinkSidebarEmphasized'?

We are also using different names when describing things. Why?

https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/598811 may render this conversation unnecessary.

Outcome

(feel free to update that page if anything seems outdated)

Event Timeline

I think the BEM name would be Sidebar__Action--Emphasized or Sidebar__Link--Emphasized with punctuation removed for file names: 'SidebarActionEmphasized', 'SidebarLinkEmphasized'.

As we are moving towards adoption of Vue.js, I find the Vue.js component naming guidelines are helpful to Mustache components as well, which go from parent->child, general->specialized:
https://vuejs.org/v2/style-guide/#Tightly-coupled-component-names-strongly-recommended

Jdlrobson triaged this task as Medium priority.Jun 21 2021, 11:24 PM
Jdlrobson moved this task from Triaged but Future to unsed on the Web-Team-Backlog board.

I think we need to have a clearance what we really want to adopt and if we need general Foundation approval. I'd reach for it.
Elements and modifiers are helpful to quickly understand CSS without knowing HTML structure too well.

Nonetheless, I think we need to clarify if the open questions raised on T255100 apply to us!
There has been no participation by Web team on that bigger task.

Jdlrobson set the point value for this task to 0.Jul 20 2021, 5:22 PM

Re: Goals

  • Decide whether we plan to ever adopt BEM and if so what our strategy for getting there is
    • While it is generally agreed that BEM is useful/beneficial for components and is already being used in WVUI (i.e. inputs), the consensus at this time is to continue with the BEM-ish convention that is currently in use in Vector over strict adherence to the BEM approach because of the following issues:
      • Difficult to enforce BEM via linting rules.
      • Solve for the grandchild element problem - flattening grandchildren seems promising but unreasonably long class names will likely become problematic.
      • Development velocity could be held up in code reviews over class naming debates.
    • Some helpful links to track the history of BEM discussions at WMF:
      • T255100 Consider CSS naming convention/methodology as coding guideline for Wikimedia projects
      • T259203 Clarify CSS naming scheme on combined components
      • Example patches outside of Vector:
        • WVUI - 639637 Limit LESS rules nesting level
        • WikibaseMediaInfo - 556050 Use BEM-style classes for input widgets
  • Agree on how to name classes in Vector currently and document in the codebase
block-element-modifier

.block {}
.block-element {}
.block-modifier {}
  • Review existing names
    • T255100#7190502 provides a good overview of how the existing BEM-ish convention is used.
  • Ensure the storybook reflects the agreed names of components that are used by designers and engineers.
    • Vector's storybook continues to be updated and includes recently-developed components that utilize this quasi-BEM structure.

Re: Outcome

The CSS/Less section of Reading/Web/Coding conventions has been updated with the latest status of BEM articulated here.

cjming removed cjming as the assignee of this task.Aug 4 2021, 9:54 PM
cjming moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
cjming subscribed.

Will discuss with the wider team on Monday before resolving, but the notes seem to cover the goals. Thanks Clare!

Change 714432 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/skins/Vector@master] Update vector menu hide dropdown class to use BEMish convention.

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

Change 714432 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Update vector menu hide dropdown class to use BEMish convention.

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

The group talked about Clare's edits to https://www.mediawiki.org/wiki/Reading/Web/Coding_conventions and we're happy with what's documented there and clear about how to write CSS going forward with the sticky header.

To avoid cargo cult programming becoming a problem, we decided to fix the class vector-menu--hide-dropdown which does not fit the coding conventions.