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

[Bug]: Flicker in AccordionItem children #9818

Closed
2 tasks done
ibandare opened this issue Oct 7, 2021 · 10 comments · Fixed by #14776
Closed
2 tasks done

[Bug]: Flicker in AccordionItem children #9818

ibandare opened this issue Oct 7, 2021 · 10 comments · Fixed by #14776
Labels
afrohacks See https://ibm.biz/afrohacks-hackathon component: accordion good first issue 👋 Used by GitHub to elevate contribution opportunities hacktoberfest See https://hacktoberfest.com/ package: react carbon-components-react role: dev 🤖 severity: 3 https://ibm.biz/carbon-severity type: bug 🐛

Comments

@ibandare
Copy link

ibandare commented Oct 7, 2021

Package

carbon-components-react

Browser

Chrome

Package version

7.45.0

Description

We have a requirement to persist the Accordion state while navigating through the application, thus kept the expand/collapsed state in the redux store. After upgrading from v7.26.0 where it worked with no issues it started blinking when being expanded or collapsed (see screen recording attached):

Screen.Recording.2021-10-07.at.21.51.37.mov

While checking on the AccordionItem sources I noticed that changing the condition from

  if (open !== prevIsOpen) {
    setAnimation(isOpen ? 'collapsing' : 'expanding');
    setIsOpen(open);
    setPrevIsOpen(open);
  }

to

if (open !== isOpen) {...

resolves the issue.

CodeSandbox example

https://codesandbox.io/s/recursing-feather-4mglx?file=/src/index.js

Steps to reproduce

Just pass the open property and update it whenever an item is expanded/collapsed.

Code of Conduct

@abbeyhrt abbeyhrt added component: accordion package: react carbon-components-react severity: 4 https://ibm.biz/carbon-severity labels Oct 8, 2021
@joshblack joshblack changed the title [Bug]: [Bug]: Flicker in AccordionItem children Oct 18, 2021
@antoinog
Copy link

antoinog commented Nov 5, 2021

Sandbox with issue. Click button , no flicker. Click header, flicker
https://codesandbox.io/s/quiet-rain-ruh6o?file=/package.json

Sandbox of 7.25 No flicker issue
https://codesandbox.io/s/cool-ride-p6o3e?file=/package.json

Possible reason:
#7876
setAnimate is called twice when header clicked, once in onclick, and once when open changes (if parent component saves state)

@ibandare
Copy link
Author

ibandare commented Dec 6, 2021

will this ever be fixed? 😢

@tay1orjones tay1orjones added severity: 3 https://ibm.biz/carbon-severity good first issue 👋 Used by GitHub to elevate contribution opportunities hacktoberfest See https://hacktoberfest.com/ and removed severity: 4 https://ibm.biz/carbon-severity labels Dec 2, 2022
@tay1orjones
Copy link
Member

To provide some context, this hasn't been addressed yet because it's lower priority. It's a cosmetic rendering issue that doesn't block the user from being able to open or close the accordion item.

Despite our best efforts every sprint, we can't get to every bug and some languish like this. I agree it's an eyesore and would like to see it fixed.

PRs are always welcome! There's some proposed solutions above that would make sense for someone to put into a PR to test out and review if interested.

@Kilian-Collender
Copy link
Contributor

Any movement on this issue?

@tw15egan
Copy link
Member

@Kilian-Collender no work has been done on this issue, but we'd welcome a PR if you have a fix in mind.

@cesardlinx
Copy link
Contributor

I wanted to explore this issue but the moment I open storybook I get this error

image

I'm following all the steps in the CONTRIBUTING.md but the moment I try to run the development server I get those errors, I'm not sure what is wrong

@tw15egan
Copy link
Member

tw15egan commented Oct 3, 2023

@cesardlinx ensure you've run yarn build from the root of the monorepo, then cd packages/react and yarn storybook

@cesardlinx
Copy link
Contributor

Hey @tw15egan thank you so much for your reply!. Yeah that was the first thing that I did but it didn't work, turns out that the one to blame was the OS, I was trying to run it on Windows and accordingly to this discussion #11042 thread there are extra steps in order to make it run on windows. But good news is that I switched to Linux and it runs smoothly! :D. I think we should consider to add that to the CONTRIBUTING.md.

Now talking about the issue, I was testing the component and it looks like this issue was already solved some time ago, I wasn't able to see any flickering, the component goes fine, only whenever I change line 151 of AccordionItem.tsx from this:
setAnimation(isOpen ? 'collapsing' : 'expanding');
to this:
setAnimation(isOpen ? 'expanding' : 'collapsing');
only then I experience flickering in the component. Maybe you should check it out

@Kilian-Collender
Copy link
Contributor

@cesardlinx which version did you try.

I just tested with the original example using the latest stackblitz and I can still see the flashing

https://stackblitz.com/edit/github-zzfu6s?file=src%2FApp.jsx

cesardlinx added a commit to cesardlinx/carbon that referenced this issue Oct 4, 2023
Removes the flickering effect when using AccordionItem as a controlled
component

Closes carbon-design-system#9818
cesardlinx added a commit to cesardlinx/carbon that referenced this issue Oct 4, 2023
Removes the flickering effect when using AccordionItem as a controlled
component

Closes carbon-design-system#9818
@cesardlinx
Copy link
Contributor

Thank you @Kilian-Collender for taking the time and for sharing that stackblitz example. Yeah! you're totally right, it helped me reproduce the problem and find the issue. Basically the flickering was only present the moment we were using it as a controlled component, but now it's fixed! :D. Here is my PR #14776

cesardlinx added a commit to cesardlinx/carbon that referenced this issue Oct 4, 2023
Removes the flickering effect when using AccordionItem as a controlled
component

Closes carbon-design-system#9818
cesardlinx added a commit to cesardlinx/carbon that referenced this issue Oct 4, 2023
Removes the flickering effect when using AccordionItem as a controlled
component
cesardlinx added a commit to cesardlinx/carbon that referenced this issue Oct 12, 2023
Removes the flickering effect when using AccordionItem as a controlled
component
@tay1orjones tay1orjones added the afrohacks See https://ibm.biz/afrohacks-hackathon label Oct 30, 2023
cesardlinx added a commit to cesardlinx/carbon that referenced this issue Nov 8, 2023
Removes the flickering effect when using AccordionItem as a controlled
component
github-merge-queue bot pushed a commit that referenced this issue Nov 30, 2023
* fix: remove flicker in AccordionItem (#9818)

Removes the flickering effect when using AccordionItem as a controlled
component

* fix: rework accordion component animation (#9818)

* refactor: use ref hook for accordion content

* test: update accordion component snapshots

* docs: fix extra missing curly brace in .all-contributorsrc

* docs: remove accordion controlled test story

---------

Co-authored-by: Andrea N. Cardona <cardona.n.andrea@gmail.com>
Co-authored-by: Alison Joseph <alison.joseph@us.ibm.com>
danoro96 pushed a commit to danoro96/carbon that referenced this issue Jan 18, 2024
…bon-design-system#14776)

* fix: remove flicker in AccordionItem (carbon-design-system#9818)

Removes the flickering effect when using AccordionItem as a controlled
component

* fix: rework accordion component animation (carbon-design-system#9818)

* refactor: use ref hook for accordion content

* test: update accordion component snapshots

* docs: fix extra missing curly brace in .all-contributorsrc

* docs: remove accordion controlled test story

---------

Co-authored-by: Andrea N. Cardona <cardona.n.andrea@gmail.com>
Co-authored-by: Alison Joseph <alison.joseph@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
afrohacks See https://ibm.biz/afrohacks-hackathon component: accordion good first issue 👋 Used by GitHub to elevate contribution opportunities hacktoberfest See https://hacktoberfest.com/ package: react carbon-components-react role: dev 🤖 severity: 3 https://ibm.biz/carbon-severity type: bug 🐛
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

9 participants