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: change transition movement and target dom(Accordion) #5431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KumJungMin
Copy link

@KumJungMin KumJungMin commented Mar 18, 2024

Suggestion

  • If I click a button while an animation is in progress, the animation is interrupted.
  • While the accordion is open, when you click the close button, the height of the accordion changes to full height.
  • The video reproduces the problem by increasing the animation time of the existing accordion.

스크린샷 2024-03-18 오후 12 22 06

2024-03-18.12.21.16.mov

  • current, Accordion animation is only css.
  • but css can not resolve temporary animation stop problem.
/* primevue/public/themes/nova/theme.css */

/* Toggleable Content */
.p-toggleable-content-enter-from,
.p-toggleable-content-leave-to {
  max-height: 0;
}

.p-toggleable-content-enter-to,
.p-toggleable-content-leave-from {
  max-height: 1000px;
}

.p-toggleable-content-leave-active {
  overflow: hidden;
  transition: max-height 0.45s cubic-bezier(0, 1, 0, 1);
}

.p-toggleable-content-enter-active {
  overflow: hidden;
  transition: max-height 1s ease-in-out;
}

Pull Request Description

So, i suggest to use requestAnimationFrame and to change transition target dom.

1. style change using requestAnimationFrame

  • First, use requestAnimationFrame on v-transition event.
enter(el) {
      el.style.maxHeight = '0';
      requestAnimationFrame(() => {
		 // set maxHeight value
          el.style.maxHeight = `${el.scrollHeight}px`;
      });
  },
  leave(el) {
     // change maxHeight to an integer value.(required to apply animation)
      el.style.maxHeight = `${el.scrollHeight}px`;
       // and, change requestAnimationFrame to 0.
      requestAnimationFrame(() => {
          el.style.maxHeight = '0';
      });
  },
  afterEnter(el) {
      // reset maxHeight value to cover resizing
      el.style.maxHeight = 'fit-content';
  },

change transition target to p-toggleable-content

  • previous, transition property is set on v-transition-event class.
  • but, this case makes transition stop problem.
// theme.css

 .p-toggleable-content-leave-active {
    overflow: hidden;
    transition: max-height 0.45s cubic-bezier(0, 1, 0, 1);
  }

  .p-toggleable-content-enter-active {
    overflow: hidden;
    transition: max-height 1s ease-in-out;
  }

  • So, i change transition target to p-toggleable-content.
    (i think, this style code should move to Accordion component)
// theme.css

.p-accordion .p-toggleable-content {
    overflow: hidden;
    transition: max-height 0.5s cubic-bezier(0, 1, 0, 1);
  }
  .p-accordion .p-toggleable-content-expanded {
    transition: max-height 0.45s ease-in-out;
  }

  • After changed code applied, We can see the issue is resolved(This video shows that by increasing the animation time)

스크린샷 2024-03-18 오후 12 31 33

-.-2024-03-18-.-12.30.53.mp4

Copy link

vercel bot commented Mar 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Visit Preview Mar 18, 2024 3:25am

@KumJungMin KumJungMin changed the title fix: change transition target dom fix: change transition move and target dom(Accordion) Mar 18, 2024
@KumJungMin KumJungMin changed the title fix: change transition move and target dom(Accordion) fix: change transition movement and target dom(Accordion) Mar 18, 2024
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.

None yet

1 participant