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

Feat/storybook atoms #405

Merged
merged 6 commits into from
Sep 7, 2021
Merged

Feat/storybook atoms #405

merged 6 commits into from
Sep 7, 2021

Conversation

ekas
Copy link
Contributor

@ekas ekas commented Aug 9, 2021

What Github issue does this PR relate to? Insert link.

What should the reviewer know?

Have all stories of atoms.

Screenshot 2021-08-09 at 19 20 20
Screenshot 2021-08-09 at 19 20 02

@ekas ekas added New feature Area/frontend [react] in the client side application labels Aug 9, 2021
@ekas ekas self-assigned this Aug 9, 2021
@ericbolikowski
Copy link
Contributor

Hi @ekas, great work here. Looks really good, so happy we're starting to document our components, and great to see them in Storybook.

I've looked through this and read up on storybook. I have quite a few questions and ideas. I heard from @helloanil that you're joining my office hours tonight? Let's go through this together then.

@ericbolikowski
Copy link
Contributor

ericbolikowski commented Aug 31, 2021

@helloanil, @ekas, I made a little discovery that can be helpful in figuring out what's going on with the <Button> styling. Please take a look and check if it's useful to resolving the issue.

https://www.loom.com/share/fba939c4d2f14c87b2c8b6d2758e19aa

@helloanil
Copy link
Contributor

@helloanil, @ekas, I made a little discovery that can be helpful in figuring out what's going on with the <Button> styling. Please take a look and check if it's useful to resolving the issue.

https://www.loom.com/share/fba939c4d2f14c87b2c8b6d2758e19aa

@ericbolikowski Great investigation and findings. I took the investigation one step further and dived deep in the code and noticed the same line @ekas has been adding in some of the files:

import 'bulma/css/bulma.min.css'

Unfortunately, this line imports bulma css as a global stylesheet for all the stories, including our Button atom. As we're using a base classname for our Button atom, button, bulma css overrides the styles for this classname as it's stylesheet is added on the top.

I've spent some time understanding if ingesting those styles is possible for only some stories, but unfortunatey that's not possible as can be seen here:

storybookjs/storybook#729 => This issue is open since Mar 2017, CRAZY!

I was going to come and comment here that we're doomed and without removing Bulma fixing this would be impossible. However, one last trick came to my mind, which is changing the base classname from .button to .button-atom. This seems to be fixing the issue pretty well as bulma css doesn't care about the button tag name but only the .button classname.

image

We still need to cut our ties with Bulma, at least in the Design System level. All our Design System components should be building blocks, supporting tokenized styling: https://styled-system.com/getting-started

Copy link
Contributor

@ericbolikowski ericbolikowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent research, explanation and final solution @helloanil 🥳 a really KISS workaround. Let's get this baby merged.

@helloanil helloanil merged commit 0cc845d into master Sep 7, 2021
@helloanil helloanil deleted the feat/storybook-atoms branch September 7, 2021 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/frontend [react] in the client side application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants