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

Class names discovery from the context ✨ #322

Open
viktor-yakubiv opened this issue Sep 11, 2020 · 3 comments
Open

Class names discovery from the context ✨ #322

viktor-yakubiv opened this issue Sep 11, 2020 · 3 comments
Labels
⚛️ components New components

Comments

@viktor-yakubiv
Copy link
Member

I’ve just come to the idea that we can simplify our components API if we utilise context technique in the library on the top level. For the instance, we have:

<Card>
  <Card.Title>I'm a card!</Card.Title>
</Card>

that turns into

<div class="card">
  <h2 class="card-title">I'm a card!</h2>
</div>

The proposal

Potentially, we can have a bit easier API like

<Card>
  <Heading>I'm a card!</Heading>
</Card>

that produces the same result.

Simple implementation

const SectionContext = React.createContext({ level: 1 })
const ClassNamesContext = React.createContext({})

const Card = ({ children, className, tag: Tag = 'div', ...restProps }) => (
  <SectionContext.Provider value={{ level: 2 }}>
    <ClassNamesContext.Provider value={{ heading: 'card-title' }}>
      <Tag className={joinClasses('card', className)} {...restProps}>
        {children}
      </Tag>
    </ClassNamesContext.Provider>
  </SectionContext.Provider>
)

const Heading = ({ children, className, ...restProps }) => {
  const level = useContext(SectionContext)?.level ?? 1
  const contextClassName = useContext(ClassNamesContext)?.heading

  const Tag = `h${level}`

  return (
    <Tag className={joinClasses(contextClassName, className)} {...restProps}>
      {children}
    </Tag>
  )
}

Use cases

My use case above is pretty simple and seems to be not worth it. I not sure either in this.

Apart from Heading replacing the Card.Title along with Section.Title we may have in the future, it looks more convenient for implicit passing classes to:

  • Icon that is descendent of the TextField, Select or Menu
  • similar to Button that is descendet of the TextField

I am pretty sure, there are other cases. However, since it's implicit, it's not obvious: very easy to use, very hard to debug.

@Joozty what do you think?

@viktor-yakubiv viktor-yakubiv added the ⚛️ components New components label Sep 11, 2020
@viktor-yakubiv
Copy link
Member Author

This can be also scaled for the headers accessibility. Check the example:

<Section id="my-section">
  <Heading>My section</Heading>
</Section>

should produce:

<section id="my-section" aria-labelledby="my-section-title">
  <h2 id="my-section-title">My section</h2>
</section>

@viktor-yakubiv
Copy link
Member Author

However, we should investigate React Context API performance. If it's bad, we should drop the idea, unfortunately. Especially interesting it the performance of combining many contexts.

@Joozty
Copy link
Member

Joozty commented Sep 14, 2020

The idea is nice. Maybe it would be worth if you created a proof of concept. The API would be simplified a lot I am just a little bit worried though it simplifies the final API it also would complicate the design development itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚛️ components New components
Projects
None yet
Development

No branches or pull requests

2 participants