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

Extending styling while using CSS Modules #178

Open
fahad19 opened this issue Sep 12, 2017 · 12 comments
Open

Extending styling while using CSS Modules #178

fahad19 opened this issue Sep 12, 2017 · 12 comments

Comments

@fahad19
Copy link

fahad19 commented Sep 12, 2017

Browsers and versions affected

n/a

Description

The problem

  • We are currently using CSS Modules for styling the Components in our custom Apps.
  • We also don't want to use the :global feature
  • There are needs sometimes for extending the styling of an embedded UI-Kit Component in our Apps
  • Extending UI-Kit component's styling requires us to know the internal CSS class names used them them
  • But if we try to write CSS selectors targeting UI-Kit Components in our Apps, they don't have any impact since CSS Modules make sure all the selectors are locally scoped (changing the class names once bundled)

Possible solution

One way we can enable this while both maintaining CSS Modules and allowing developers to extend the UI-Kit Components in their Apps is by supporting a className prop in all UI-Kit components.

// my/custom/app/components/MyComponent.js
import React from 'react';
import { Button } from 'travix-ui-kit';

import styles from './styles.css';

export default function MyComponent(props) {
  return (
    <div>
      <h1>Hello World</h1>

      <Button className={styles.fancyButton}>
        Click me!
      </Button>
    </div>
  );
}

And the styles in a CSS Module can be:

/* my/custom/app/components/styles.css */
.fancyButton {
  color: tomato;
  letter-spacing: 500px;
}

Steps to reproduce

  • Use CSS Modules (with :global disabled)
  • Try extending the styling of any UI-Kit Component
  • 🤔

Expected results

Being able to extend the styling of UI-Kit components shouldn't require knowing the CSS class names used by the Components internally.

And developers should be allowed to do so without being able to modify the styling of UI-Kit elsewhere. Only their own Apps are affected by their own code.

Actual results

n/a

@asci
Copy link
Contributor

asci commented Sep 12, 2017

  1. Introducing fancyButton leads to visual inconsistent in UI.
    If you need something like this probably:
  • you need to discuss this with designers and change design to contain button from UI kit
  • or you need to add a new variation to UI kit
    We also have propmods. But I found only 2 places in RWD repo we're actually using mods
  1. Is there a solution if you don't disable :global?

@fahad19
Copy link
Author

fahad19 commented Sep 12, 2017

  • fancyButton is an example use case
  • mods won't work with CSS Modules either, because you need to know the prefix of the UI-Kit Component's CSS class names
  • :global usage in CSS Modules will be banned for App developers, because we don't want to risk others being able to change the styling of our whole website

@asci
Copy link
Contributor

asci commented Sep 12, 2017

example of fancyButton is typical and the idea applicable not only to fancyButton

@mAiNiNfEcTiOn
Copy link
Contributor

@fahad19 is this the effective issue related to #153 ?

@fahad19
Copy link
Author

fahad19 commented Sep 12, 2017

@mAiNiNfEcTiOn: they are separate.

Issue #153 is about using postcss-travix for handling the CSS bundling, so we can benefit from one single config everywhere.

This issue is a feature request for the Components (JS) in UI-Kit.

@asci
Copy link
Contributor

asci commented Sep 12, 2017

so, any thoughts how can we prevent inconsistency of UI when we introduce classNames (instead of mods)?

@fahad19
Copy link
Author

fahad19 commented Sep 12, 2017

enabling className prop would allow other developers to extend the styling of UI-Kit components, no doubt.

but with CSS Modules and :global disabled, we at least know that they can only mess up their own Apps, not someone else's. It will be contained.

if there happens to be inconsistencies, manual reviews would be needed.

I suggest establishing something like design guidelines for Travix, like how Ant Design does here for example: https://ant.design/docs/spec/introduce

Enable developers to do things. But guide them about what to do, and especially what not to do.

@yurist38
Copy link
Contributor

yurist38 commented Sep 12, 2017

Here is the case: I'm working on seatmap widget right now. I have HTML structure built by amadeus script, that structure contains static CSS classes what I need to style. What if we allow using global css class names only inside of generated ones? So we will be sure that we are override/extend only content inside of our parent (widget/block).

@mAiNiNfEcTiOn
Copy link
Contributor

I agree with @fahad19, exposing className as an attribute would be a good way to go.

Also, I would add that:

  • That className should only be used on the root node or passed to a root node of a sub-component, not used on sub HTML Elements generated internally:
// this would be ok:

render() {
  const { className } = this.props;
  return (
    <div className={className}>
      bla
    </div>
  );
}

// or...

import SubComponent from './SubComponent'; // and internally this would behave as the previous example

/*...*/

render() {
  const { className } = this.props;
  return (
    <div>
      <SubComponent className={className} />
    </div>
  );
}



// this would NOT be ok

render() {
  const { className } = this.props;
  return (
    <div>
      <div className="other-stuff">
        <h3  className={className}>Bla</h3>
      </div>
    </div>
  );
}

If you have the need to have a className in an inner element generated by you, either extract it as a component to pass the attribute (which in this last case would make no sense to do an H3 component, so just do proper CSS :D)

@asci
Copy link
Contributor

asci commented Sep 14, 2017

So, guys, you're not even considering that you may not need classNames at all :)? As I said we have only 2 places in our codebase where we use mods. And those cases could\should go to UI kit itself.

The idea is to have standard UI. If you need to customize it — add it to UI kit, so other developers will be able to reuse it. Or talk to designers and change the design to use components from the library. If even this is not the case — you may not need UI kit at all for your component.

Let's wait for @iwwwi before taking action on this

@mAiNiNfEcTiOn
Copy link
Contributor

@asci sure... still some discussion never harmed anyone.

Can you elaborate more on the

you may not need classNames at all :)?

? Since we're discussing the possibility of using CSS Modules on UI Kit, using classNames would definitely be a need... And perhaps a replacement for BEM's methodology?

@asci
Copy link
Contributor

asci commented Sep 14, 2017

@mAiNiNfEcTiOn
using CSS Modules inside UI kit doesn't require to expose it to public API.

you may not need classNames at all

I tried to elaborate on this in my message
Let's try from the other side — when do you expect to use classNames?

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

No branches or pull requests

4 participants