Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Components: remove hardcoded classnames and move away from Sass styles #43083

Open
ciampo opened this issue Aug 9, 2022 · 0 comments
Open

Components: remove hardcoded classnames and move away from Sass styles #43083

ciampo opened this issue Aug 9, 2022 · 0 comments
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@ciampo
Copy link
Contributor

ciampo commented Aug 9, 2022

Context

Hardcoded classnames have been historically used in the @wordpress/components package as the main way to style component while writing styles in Sass.

But while hardcoded classnames have (almost) never been intended to be part of the public API of a component, they've been often used by consumers of the @wordpress/components package (both in the Gutenberg repo, and outside of it) as a way to override the components' styles with custom styles.

As @nerrad pointed out, the project does have existing documentation about backwards compatibility around class names and DOM updates.

Class names and DOM nodes used inside the tree of React components are not considered part of the public API and can be modified.
Changes to these should be done with caution as it can affect the styling and behavior of third-party code (Even if they should not rely on these in the first place). Keep the old ones if possible. If not, document the changes and write a dev note.

This practice, while allowing infinite customization options, is actually quite bad for the components library itself: styling a component using its internal, hardcoded classnames (or by relying on its implicit DOM structure) is an extremely fragile technique, prone to breakage as soon as a component's implementation details change.

And since we, maintainers of the library, are very mindful about not causing breaking changes, the more hardcoded classnames are used the harder it is for us to improve components iteratively without causing such breaking changes.

The added benefit of using CSS-in-JS is better style encapsulation compared to the current Sass styles (although I am aware that there are other technologies that also offer style encapsulation, but that's a different topic)

The plan

For each component still using .component-* classnames:

  1. Take note of all the hardcoded classnames exposed by the component
  2. If the components' styles are written in Sass, convert the component to be styled via CSS-in-JS (see the current guidelines for more info)
  3. Remove all usages of the hardcoded classnames that are not listed as public API (i.e. in the README), even from other components in the @wordpress/components package
    • If possible, ideally we should aim at removing as many style overrides as possible — either by leveraging better component composition, recent features, or finding alternative solutions.
    • Based on the need that the style overrides highlight, we may even decided to add some features directly in the component's source
    • If overriding styles is still necessary, instead of using the component's hardcoded classname, the consumer of the component should be passing a new classname (via the className prop) and use it as a way to assign custom styles to the component.
    • This will potentially flag a few instances where style overrides can't be totally replaced by non-hacky styles. I will argue that the most important thing is to make sure that the component's hardcoded classname is no longer used.
  4. Remove the remaining instances of the hardcoded classnames from the source component, and make sure that there's no trace of these hardcoded classnames from the Gutenberg repository.

We should also update the CONTRIBUTING guidelines.

Risks

As well summarized by @mirka in #42789 (comment)

Could it cause breakage? Yes. Should we consider it a Breaking Change? No.

I was a consumer of the wp-components package before coming to the maintainer side, and I was never under the impression that CSS classnames (or styling, or DOM structure, for that matter) were guaranteed not to change. I think most people realize that overriding internal styling is fragile, and that it's their responsibility to protect against breakage if they are going to do so. Unless these classnames are publicly documented, as in the examples you raised 😅

Removing hardcoded classnames and reducing the amount of style overrides will also give us a better idea of how components are used "in the wild", and potentially inform future decisions.

@ciampo ciampo added the [Package] Components /packages/components label Aug 9, 2022
@ciampo ciampo added [Type] Enhancement A suggestion for improvement. [Type] Code Quality Issues or PRs that relate to code quality [Type] Overview Comprehensive, high level view of an area of focus often with multiple tracking issues labels Aug 9, 2022
@annezazu annezazu added [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. and removed [Type] Overview Comprehensive, high level view of an area of focus often with multiple tracking issues labels Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Development

No branches or pull requests

2 participants