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.

Sign up for GitHub

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

Jump to bottom

Docs: remove theme attribute from demo html and stories, use classes #2262

Closed
caripizza opened this issue Jun 4, 2021 · 6 comments
Closed

Docs: remove theme attribute from demo html and stories, use classes #2262

caripizza opened this issue Jun 4, 2021 · 6 comments
Assignees
@caripizza
Labels
4 - verified Issues that have been released and confirmed resolved. docs Issues relating to documentation updates only.
Milestone
Sprint 6/7 – 6/18

Comments

@caripizza
Copy link
Contributor

Description

This is a follow on to #2189. Now that pr #2194 is in, and we are deprecating the usage of theme="light" and theme="dark" attributes for hooking into a theme, we need to update our resources:

  1. remove theme attribute from demo html pages
  2. remove theme attribute from component stories

Refactors should instead use the new .calcite-theme-light or .calcite-theme-dark classes for setting deliberate themes on components.

(Once we have these resources updated, we'll then add documentation on how to use the light/dark themes via the new classes.)

Which Component

  • All component demo html pages
  • All component stories

Resources

cc @driskull
@caripizza caripizza added docs Issues relating to documentation updates only. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Jun 4, 2021
@julio8a julio8a added this to the Sprint 6/7 – 6/18 milestone Jun 4, 2021
@julio8a julio8a removed the needs triage Planning workflow - pending design/dev review. label Jun 4, 2021
@driskull driskull mentioned this issue Jun 4, 2021
Merged
@driskull
Copy link
Member

driskull commented Jun 4, 2021

For the stories, how do we want to proceed? Do we want a knob for the theme class? Or do we just want each story to have another "dark" theme story?

@caripizza
Copy link
Contributor Author

I vote actual dark theme story for Screener screenshot tests if we can (like the RTL ones)

@caripizza
Copy link
Contributor Author

Do you think it'd be good to keep the existing theme knobs in place too?

@driskull
Copy link
Member

driskull commented Jun 4, 2021

I'm not sure about the knobs. We would have to use a class which is kind of an odd knob.

@driskull
Copy link
Member

driskull commented Jun 5, 2021

I'm wondering if we should just remove both knobs for theme and dir and just have something to automatically generate dark and RTL stories for every story we create? This would give screener better coverage and be more automatic so that we only have to create unique stories. It would have to automatically export a const for dark/RTL

What do you think @jcfranco?

@driskull driskull self-assigned this Jun 7, 2021
driskull added a commit that referenced this issue Jun 7, 2021
@driskull
feat!: Use HTML classes instead of custom 'theme' attributes for them…
b762520 
…ing components. #2262

BREAKING CHANGE: Using a 'theme' attribute is no longer supported to theme a component. Instead, a HTML class attribute of 'calcite-theme-light', 'calcite-theme-dark', or 'calcite-theme-auto' should be set on a component or one of its parent elements. Example: `<calcite-button theme="light" />` now becomes `<calcite-button class="calcite-theme-light" />`. The `calcite-theme-auto` class will defer to the browser's CSS 'prefers-color-scheme' media query to decide whether the light or dark theme will be used.
@driskull driskull mentioned this issue Jun 7, 2021
Merged
@driskull driskull added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Jun 7, 2021
driskull added a commit that referenced this issue Jun 7, 2021
@driskull
feat!: Use HTML classes instead of custom 'theme' attributes for them…
ce3ccd5 
…ing components (#2271)

* feat!: Use HTML classes instead of custom 'theme' attributes for theming components. #2262

BREAKING CHANGE: Using a 'theme' attribute is no longer supported to theme a component. Instead, a HTML class attribute of 'calcite-theme-light', 'calcite-theme-dark', or 'calcite-theme-auto' should be set on a component or one of its parent elements. Example: `<calcite-button theme="light" />` now becomes `<calcite-button class="calcite-theme-light" />`. The `calcite-theme-auto` class will defer to the browser's CSS 'prefers-color-scheme' media query to decide whether the light or dark theme will be used.

* fix screener tests and utils
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Jun 7, 2021
@driskull driskull assigned caripizza and unassigned driskull Jun 7, 2021
@caripizza
Copy link
Contributor Author

caripizza commented Jun 8, 2021 •
edited by driskull
Loading

Need to remove theme attribute from these files too:

caripizza added a commit that referenced this issue Jun 8, 2021
@caripizza
docs: remove final theme attribute from demo html, readme (#2262)
2183378
@caripizza caripizza mentioned this issue Jun 8, 2021
Closed
driskull added a commit that referenced this issue Jun 8, 2021
@driskull
docs: Update documentation for theming components and fix demo toggli…
d80769a 
…ng theme. #2262
@driskull driskull mentioned this issue Jun 8, 2021
Merged
driskull added a commit that referenced this issue Jun 8, 2021
@driskull
docs: Update documentation for theming components and fix demo toggli…
4de2e27 
…ng theme. #2262 (#2279)
@caripizza caripizza added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@caripizza caripizza

Labels
4 - verified Issues that have been released and confirmed resolved. docs Issues relating to documentation updates only.
Projects
None yet
Milestone
Sprint 6/7 – 6/18
Development

No branches or pull requests
3 participants
@driskull @julio8a @caripizza