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

Enhancement: Remove styles that directly hook into a theme attribute #1737

Closed
driskull opened this issue Mar 11, 2021 · 5 comments
Closed

Enhancement: Remove styles that directly hook into a theme attribute #1737

driskull opened this issue Mar 11, 2021 · 5 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. enhancement Issues tied to a new feature or request. p - high Issue should be addressed in the current milestone, impacts component or core functionality

Comments

@driskull
Copy link
Member

Description

I think we'll need to remove styles that directly hook into a theme attribute as well, as that will no longer happen. A quick search reveals the following:

src/components/calcite-alert/calcite-alert.scss:
  7  // dark theme
  8: :host([theme="dark"]) {
  9    --calcite-alert-dismiss-progress-background: #{rgba($blk-200, 0.8)};

src/components/calcite-button/calcite-button.scss:
  11  // dark theme
  12: :host([theme="dark"]) {
  13    --calcite-button-transparent-hover: rgba(255, 255, 255, 0.05);

src/components/calcite-chip/calcite-chip.scss:
    9  // dark theme
   10: :host([theme="dark"]) {
   11    --calcite-chip-transparent-hover: rgba(255, 255, 255, 0.05);

  173    // handle dark theme text accessibility for solid
  174:   :host([theme="dark"][color="#{$name}"][appearance="solid"]:not([color="grey"])),
  175:   :host([theme="dark"][color="#{$name}"][appearance="solid"]:not([color="grey"])) button {
  176      color: var(--calcite-ui-background);

  187  
  188:   :host([theme="dark"][color="#{$name}"][appearance="clear"]) {
  189      border-color: $bgColorClearAppearanceDark;

src/components/calcite-inline-editable/calcite-inline-editable.scss:
  66  :host(:not([editing-enabled])) {
  67:   &:not([theme="dark"]) {
  68      &::slotted(*) {

src/components/calcite-input-message/calcite-input-message.scss:
  20  
  21: :host([theme="dark"]) {
  22    --calcite-input-message-floating-background: #{rgba($ui-foreground-1-dark, 0.96)};

src/components/calcite-link/calcite-link.scss:
  8  // dark theme
  9: :host([theme="dark"]) {
  10    --calcite-link-blue-underline: #{rgba($d-bb-420, 0.4)};

src/components/calcite-scrim/calcite-scrim.scss:
  8  // dark theme
  9: :host([theme="dark"]) {
  10    --calcite-scrim-background: #{rgba($blk-240, 0.75)};

These vars will need to be moved to the actual global theme variables, I think.

Acceptance Criteria

No more variables hooking into theme attribute

Relevant Info

#1735 (comment)

Which Component

All

Example Use Case

trying to make it easier to switch from light/dark without having to do so by applying a prop on every component.

@driskull driskull added enhancement Issues tied to a new feature or request. p - high Issue should be addressed in the current milestone, impacts component or core functionality 1 - assigned Issues that are assigned to a sprint and a team member. labels Mar 11, 2021
@driskull
Copy link
Member Author

The colors in the above look largely made up, so would be good to standardize those anyways...

@driskull
Copy link
Member Author

@julio8a should this be part of the refactoring of tailwind?

@driskull
Copy link
Member Author

It would be nice if these component variables were moved to be global variables so that the host element doesn't need to have a theme attribute applied.

@julio8a
Copy link

julio8a commented Apr 29, 2021

@caripizza, can you review this along with the outstanding refactor work we have?

@caripizza caripizza assigned caripizza and unassigned julio8a Apr 29, 2021
@caripizza caripizza added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels May 3, 2021
@caripizza caripizza added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. 4 - verified Issues that have been released and confirmed resolved. and removed 2 - in development Issues that are actively being worked on. 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels May 17, 2021
@caripizza
Copy link
Contributor

Verified on my local master branch by searching for theme= in *.scss files. The only results now are:

2 results - 1 file

src/assets/styles/global.scss:
  32  
  33: [theme="dark"] {
  34    @include calcite-theme-dark();

  46  
  47: [theme="light"] {
  48    @include calcite-theme-light();

Verified and closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. enhancement Issues tied to a new feature or request. p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Development

No branches or pull requests

3 participants