Skip to content
This repository has been archived by the owner on Aug 30, 2019. It is now read-only.

[General Discussion] Are internal class names considered public API? #107

Open
reaktivo opened this issue Jun 23, 2017 · 5 comments
Open

Comments

@reaktivo
Copy link
Contributor

So my use case is the following: I'm working with the Modal component, when I want to set a header, the component will automatically overwrite my passed in header's className property to use ui-modal__header, this css class has a fixed min-height: 120px attached to it.

This gives me some options:

First option

To pass a mod into the Modal component that I'm rendering and via a really specific css selector, overwrite this property. Since none of the internal class names are documented in any way, they seem subject to change and break my selectors. I've also heard moving to CSS Modules is being considered, this would mean that we can't rely on the same css selectors that our stylesheets are using.

/* In my component.js */
<Modal mods=["my-modal"] />

/* In my style.css */
.ui-modal-my-modal .ui-modal__header {
  /* hope the previous selector is more specific */
  min-height: 0;
  padding: 0;
  margin: 0;
}

Second Option
To modify the Modal component to allow passing in my own class names for sections of the component:

/* In my component.js */
<Modal headerClassName="my-modal-header"/>

/* In my style.css */
.my-modal-header {
  min-height: 0;
}

I want to start a discussion about the following subjects:

  • Are internal css classes considered public API's, in a way that we can expect them to remain stable? If so, how should we document them.
  • If we rely on css classes as hooks for customisation, we also depend on the html markup hierarchy, which is fragile.
  • When and if it's decided to introduce a breaking change, can we switch to relying on the component's displayName property when creating "section" classNames? This is less error prone.
@asci
Copy link
Contributor

asci commented Jun 23, 2017

I would prefer the first option, but docs question is open.

But since we're going to use CSS Modules I guess the second option is better.

Third option

Another option is to provide data attributes to elements (E from BEM), so even if we will switch to CSS Modules they will still remain. and you will write something like:

/* In my component.js */
<Modal mods=["my-modal"] />

/* In my style.css */
.ui-modal_my-modal [elem=header] {
  min-height: 0;
}

@reaktivo
Copy link
Contributor Author

Third option sounds good to me, we can clearly separate what is intended for internal use and what is intended as a hook for customisation. This also allows us to move along without breaking changes. The only con being I guess is having data-elem being thrown around.

@reaktivo
Copy link
Contributor Author

Any thoughts @mAiNiNfEcTiOn @EduardTrutsyk @iwwwi ?

@asci
Copy link
Contributor

asci commented Jun 26, 2017

Any thoughts @mAiNiNfEcTiOn @EduardTrutsyk @iwwwi ?

@iwwwi
Copy link
Contributor

iwwwi commented Aug 8, 2017

@reaktivo

  1. (in this particular case) I think that using this library should bring us to the level where we should have less overriding on our website, meaning that if by any chance the styles introduced here are different from what you have in your design files there are 2 possible points that are not synchronised. Either designs are wrong or the general styles must be updated (ui kit itself) for the whole website (remember the purpose of this library was to unify styles around the websites).
  2. Upper mentioned comment (1.) doesn't discard the question you have regarding the class documentation. I also think the class name changes should be treated as major releases which might bring breaking changes. In my opinion these should most of the time stable and should not change very often.
  3. Adding css modules as part of the ui kit should be investigated closer and check the impact from its implementation.
  4. We can add all this suggestions for voting and decide what is the best approach.
    cc: @asci

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants