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

fix: error at color-system story #378

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shootermv
Copy link
Contributor

@shootermv shootermv commented Oct 14, 2024

not sure why, but things start working after renaming color.css to colors.module.css:

Closes #66

@shootermv shootermv requested a review from a team as a code owner October 14, 2024 14:10
Copy link
Member

@huyenltnguyen huyenltnguyen left a comment

Choose a reason for hiding this comment

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

Thank you for the PR.

There are a couple of issues with the changes that I don't think we can merge as-is:

  • colors.css still exists, and its import aren't updated.
  • We are using globally-scoped stylesheets in the codebase, if we are changing this to a module, other stylesheets should follow.
  • We would like to understand the root cause of the issue (whether this is an issue in the repo setup or in the storybook setup) in order to update the config accordingly.

@huyenltnguyen huyenltnguyen added the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Oct 15, 2024
@shootermv
Copy link
Contributor Author

shootermv commented Oct 15, 2024

Thank you for the PR.

There are a couple of issues with the changes that I don't think we can merge as-is:

  • colors.css still exists, and its import aren't updated.
  • We are using globally-scoped stylesheets in the codebase, if we are changing this to a module, other stylesheets should follow.
  • We would like to understand the root cause of the issue (whether this is an issue in the repo setup or in the storybook setup) in order to update the config accordingly.

i have impression that problem is not the storybook,
seems to me the problem is importing css variables into javascript as object
(import colorList from "../colors.css"; at colors-system component)
for some reason it works with modules an not with genral css...
But! we can use module file only for color-systems and import the colors.css into it using @import ...

@shootermv
Copy link
Contributor Author

is the PR is blocked?

@huyenltnguyen
Copy link
Member

is the PR is blocked?

Yes. The changes haven't addressed my concerns in #378 (review).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Color story is not rendering
2 participants