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(theme): correct emphasis opacity in default dark theme #17279

Merged
merged 1 commit into from
May 3, 2023

Conversation

douira
Copy link
Contributor

@douira douira commented May 3, 2023

Matched the opacity of the emphasis CSS variables to conform to the values stated in the documentation.

If this change is not wanted, the documentation should instead be updated to explain that there is no difference in opacity between the light and dark theme.

Description

The documentation states different opacity values for high, medium and disabled opacity text between light and dark mode. However, the theme only uses the values of the light theme. I've set the correct values as stated in the documentation for the dark theme (and the corresponding snapshot test).

The PR that added these variables: https://github.com/vuetifyjs/vuetify/pull/13674/files#diff-53b5523de35fefbb957b3194ca376f52e49b1562b8715abbeec14437966fb0c6

Markup:

<template>
  <v-app>
    <v-container>
      <v-card theme="light">
        <v-card-text>
          light theme default (high emphasis)
          <div class="text-high-emphasis">high emphasis</div>
          <div class="text-medium-emphasis">medium emphasis</div>
          <div class="text-disabled">disabled emphasis</div>
        </v-card-text>
      </v-card>
      <v-card theme="dark">
        <v-card-text>
          dark theme default (high emphasis)
          <div class="text-high-emphasis">high emphasis</div>
          <div class="text-medium-emphasis">medium emphasis</div>
          <div class="text-disabled">disabled emphasis</div>
        </v-card-text>
        <v-card-text class="old-emphasis">
          dark theme before this PR (emphasis like in light theme)
          <div class="text-high-emphasis">high emphasis</div>
          <div class="text-medium-emphasis">medium emphasis</div>
          <div class="text-disabled">disabled emphasis</div>
        </v-card-text>
      </v-card>
    </v-container>
  </v-app>
</template>

<script>
  export default {
    name: 'Playground',
    setup () {
      return {
        //
      }
    },
  }
</script>

<style>
.old-emphasis {
  --v-high-emphasis-opacity: 0.87;
  --v-medium-emphasis-opacity: 0.6;
  --v-disabled-opacity: 0.38;
}
</style>

Matched the opacity of the emphasis CSS variables to conform to the values stated
in the documentation.
@douira
Copy link
Contributor Author

douira commented May 3, 2023

How do I get cypress to update the snapshot data? I can’t find any documentation on how contributors should use the testing infrastructure.

@KaelWD
Copy link
Member

KaelWD commented May 3, 2023

That specific failure is just a cypress bug, you can ignore it: cypress-io/cypress#25913

@johnleider johnleider changed the title fix: Correct emphasis opacity for dark mode fix(theme): change default opacity for dark May 3, 2023
@johnleider johnleider added T: bug Functionality that does not work as intended/expected E: theme Theme composable labels May 3, 2023
@johnleider johnleider added this to the v3.x.x milestone May 3, 2023
@KaelWD KaelWD modified the milestones: v3.x.x, v3.2.x May 3, 2023
@KaelWD KaelWD changed the title fix(theme): change default opacity for dark fix(theme): correct emphasis opacity in default dark theme May 3, 2023
@KaelWD KaelWD merged commit 1f6b2b3 into vuetifyjs:master May 3, 2023
@douira douira deleted the fix/dark-theme-opacity branch May 3, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E: theme Theme composable T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants