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

Proposal/RFC convention that mimics CSS parts and state #1030

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

tomalec
Copy link
Member

@tomalec tomalec commented Sep 29, 2021

Changes proposed in this Pull Request:

This is an RFC & POC of a CSS classes convention, to make React components, mimic native CSS behavior of :part and :--state (Custom Pseudo Class).

Despite the syntactic sugar, the main take here is that the component author specifies the parts/areas/classes as a part of public API, for external CSS styling. Then a consumer does not need to dig into the source code of imported components and rely on implementation details.

Benefits:

  • less fragile code base, as we don't rely on volatile implementation details, but exposed API
  • faster development, as we don't need to dig through the source code over and over to find suitable classes to hack
  • faster development, as it's easier to distinguish a local class from an internal class of the external components
  • the person who decides which class of the external component is safe to use, is the external component author who is more informed for such a decision
  • syntactic sugar

Drawbacks:

  • not as semantic, established, unopinionated, well tested, scoped, as native CSS, but well… React.
  • component authors need to maintain CSS classes as API - maintainability is distributed across all consumers, which is even worse.

Detailed test instructions:

  1. Read the code
  2. See the beauty
  3. See the evil
  4. Leave your feedback

Additional

  • In the CSS comments, for reference, I added the selectors that we could use with native HTML (custom) elements.
  • I don't push for any particular class name/attribute name convention. I just think it would be nice to have a ::part, :-- equivalent for React components. (:host, element-name, & ::slotted would be great as well).

@tomalec tomalec added the type: enhancement The issue is a request for an enhancement. label Sep 29, 2021
Base automatically changed from feature/962-separate-settings-phone to develop September 30, 2021 13:36
@tomalec tomalec self-assigned this Sep 30, 2021
@eason9487
Copy link
Member

Not sure if you mean that the components all use the same naming, such as the part-- used in this RFC, if so I'm a little concerned. In order to reduce the possibility of global CSS naming collisions that might cause conflicts when using CSS selectors, I think the prefix part-- still has to be the CSS name of the main component. Such as gla-account-card--. And the same concept goes for the state--.

@tomalec
Copy link
Member Author

tomalec commented Oct 4, 2021

Not sure if you mean that the components all use the same naming, such as the part-- used in this RFC,

In the perfect case, I would like all to use just part-- to avoid verbosity and enforce readability. But given the limitations, I'm open to adapt it.

In order to reduce the possibility of global CSS naming collisions that might cause conflicts when using CSS selectors, I think the prefix part-- still has to be the CSS name of the main component. Such as gla-account-card--. And the same concept goes for the state--.

In the native case, the collisions are avoided automatically, as we got scoping for free. Just by the fact, it's a CSS pseudo element

A CSS pseudo-element is a keyword added to a selector that lets you style a specific part of the selected element(s).

So the part selector is never used alone. It's used with some other selector, to match witin the selected element.

To mimic that behavior for React components, we could either enforce policy, to:

  1. never use .part-- selector alone
    That's cleaner code, and syntactically closer to the native.
    But, is more fragile, harder to enforce, and still may conflict, if some third party, beyond our control, but within the page happen to use the same name.
  2. always prefix with the component name
    More verbose, and blurry.
    But closer to BEM convention, and makes it easier to avoid conflicts in non-scoped React's CSS.

@eason9487
Copy link
Member

eason9487 commented Oct 8, 2021

I know it's not a problem in native case, but what I meant about the concern is that mimics ::part with CSS classes convention.

So the part selector is never used alone. It's used with some other selector, to match witin the selected element.

To mimic that behavior for React components, we could either enforce policy, to:

  1. never use .part-- selector alone
    That's cleaner code, and syntactically closer to the native.
    But, is more fragile, harder to enforce, and still may conflict, if some third party, beyond our control, but within the page happen to use the same name.

Even though using another name instead of part-- can avoid conflict with third-party styles, we still face CSS name collisions with GLA codebase itself.

For example, basic components and composite components for share usage, and a consumer side component.

function Icon() {
  return <img className="part--icon" />;
}

function Item() {
  return (
    <ul className="gla-item">
      { /*<Icon />*/ }
      <div className="part--title" />
      <div className="part--description" />
    </ul>
  );
}

function List( { items } ) {
  return (
    <div className="gla-list">
      <Icon />
      <div className="part--title" />
      <li>
        { items.map( () => (
          <Item />
        ) ) }
      </li>
    </div>
  );
}

function Page() {
  return (
    <div className="some-page">
      <Icon />
      <div className="part--description" />
      <List />
    </div>
  );
}

Possible styling problems:

.some-page {
  .part--description {
    // Want to style page's description but also affect items' description
  }

  .part--title {
    // Attempt to style title of either list or item without looking into the implementation details,
    // but affects another one.
  }

  .gla-list {
    .part--icon {
      // Styled list's icon correctly without affecting to page's icon.

      // But it might still break something if one day the <Icon> is added to <Item>,
      // such as the commented out part within <Item>.
    }
  }
}

I think that is why the convention of prefix component names like BEM is necessary.

@puntope
Copy link
Contributor

puntope commented Nov 18, 2021

I'm trying to understand the real issue from within this proposal comes. It's just about conflicts?

I was reading about the history of the project in order to get more context. I would appreciate a briefing about the context, otherwise, I will read all the related PR's little by little.

Re- CSS Conflicts I used this (implemented by default in NextJS)

https://github.com/css-modules/css-modules

Never had a CSS conflict since I use it. And there are kinda default also in React Apps World (I know you don't 100% like it but that is where we are).

Check some use cases here: https://css-tricks.com/css-modules-part-1-need/#what-are-css-modules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants