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(VRipple): use $ripple-animation-visible-opacity variable #14365

Merged
merged 3 commits into from
Nov 30, 2021
Merged

fix(VRipple): use $ripple-animation-visible-opacity variable #14365

merged 3 commits into from
Nov 30, 2021

Conversation

KareemDa
Copy link
Contributor

@KareemDa KareemDa commented Nov 2, 2021

Description

fixes #11505

  • removed variables from packages/vuetify/src/directives/ripple/_variables.scss as they are already defined in packages\vuetify\src\styles\settings\_variables.scss
  • removed opactiy functionality from Vripple index.ts as it overrides the opactiy in sass file
  • added opacity variable, functionality into sass file

Motivation and Context

How Has This Been Tested?

Markup:

<template>
  <v-app id="inspire">
    <div v-ripple class="text-center elevation-2 pa-12 text-h5">
      HTML element with v-ripple
    </div>
  </v-app>
</template>

<script>
export default {};
</script>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@KareemDa KareemDa changed the title fixes #11505 fix: #11505 Nov 2, 2021
$ripple-animation-transition-in: transform .25s map-get($transition, 'fast-in-slow-out'),
opacity .1s map-get($transition, 'fast-in-slow-out') !default;
$ripple-animation-transition-out: opacity .3s map-get($transition, 'fast-in-slow-out') !default;
$ripple-animation-visible-opacity: .15 !default;
Copy link
Member

@KaelWD KaelWD Nov 3, 2021

Choose a reason for hiding this comment

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

Prefer this file over the global one if the variables aren't used anywhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've thought about that. but the problem is that it won't be documented! if it's written only in ripple variable.scss.
directives are not mentioned in Sass variables page

If I keep it in global variable it'll be added to $vuetify object

Copy link
Member

Choose a reason for hiding this comment

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

Ok remove this file entirely then if it does nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@KareemDa KareemDa changed the title fix: #11505 fix(VRipple): Enable use ignored SASS Variables Nov 23, 2021
@KareemDa KareemDa changed the title fix(VRipple): Enable use ignored SASS Variables fix(VRipple): Use ignored SASS Variables Nov 23, 2021
@KaelWD KaelWD changed the title fix(VRipple): Use ignored SASS Variables fix(VRipple): use $ripple-animation-visible-opacity variable Nov 30, 2021
@KaelWD KaelWD merged commit 3f8f577 into vuetifyjs:master Nov 30, 2021
@KareemDa KareemDa deleted the fix/11505 branch November 30, 2021 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] $ripple-animation-visible-opacity ignored
2 participants