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

feat: Add fade transition for cookie options #56

Closed
wants to merge 1 commit into from
Closed

feat: Add fade transition for cookie options #56

wants to merge 1 commit into from

Conversation

alexjharrison
Copy link
Contributor

Description

  • The v-if'd div of checkboxes have been wrapped in a transition tag
  • A vue transition class called slide-fade was added to define how it should enter and exit

The main issue is that I hardcoded the starting height of the div during the transition to 44px.
https://github.com/alexjharrison/vue-cookie-consent-banner/blob/20-cookie-options-transition/src/components/CookieModal.vue#L276

Setting to height: 100% broke the transition and reading the height with JS reports a height of 0px when in a closed state. Not sure how to handle this when the slot is overwritten.


Issue Ticket Number

Fixes #20


Type of change

  • 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)
  • This change requires a documentation update

Checklist:

  • I have followed the contributing guidelines of this project as mentioned in CONTRIBUTING.md
  • I have checked to ensure there aren't other open Pull Requests for the same update/change?
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes needed to the documentation

@alexjharrison alexjharrison marked this pull request as draft October 5, 2022 22:29
@alexjharrison
Copy link
Contributor Author

Just realized I started with a very old branch. Will rework this.

@Gismo1337
Copy link
Owner

sorry to hear that @alexjharrison
is this my fault? I am new at maintain a repo too, so may I had to inform you?

@alexjharrison
Copy link
Contributor Author

No this was on me. I made the fix on my original fork and didn't update to the latest master which had many more changes.

My fix mostly works but I'm working on edge cases for different screen sizes. I should have another PR tomorrow.

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.

Transition for open and close cookie options
2 participants