Skip to content

Fix #4158 #4048: sidebar buggy when animations disabled #4203

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

Closed

Conversation

palmerusaf
Copy link
Contributor

This reverts commit 71183da.

Description

Describe what this change achieves

This PR fixes a regression that causes sidebar layout issues when animations are disabled. After some research I know for sure the regression happened inside the angular/components library. The PR simply downgrades those dependencies by reverting the commit that caused the regression. I haven't pinpointed where in the angular library this happened, but reading the logs between versions points to this refactor being a likely candidate.

Issues Resolved

List any existing issues this PR resolves

Check List

  • New functionality includes testing.
  • [n/a] New functionality has been documented in the README if applicable.

@dosubot dosubot bot added bug dependencies Pull requests that update a dependency file labels Apr 3, 2025
@palmerusaf
Copy link
Contributor Author

I noticed one of the e2e tests is failing. I had some trouble with this test locally as well. It was even failing on master branch. I was able to get all e2e passing by rerunning. I also did a quick test build on android with no issues. Additionally, I've been using a desktop build with this commit for a week with no issues.
image

@johannesjo
Copy link
Owner

Sorry about the e2e tests. This happens sometimes depdending on the time of day and the timezone the server is in (I think).

About the fix: I suppose upgrading to 19.2.8 instead, does not fix the issue?

@palmerusaf
Copy link
Contributor Author

@johannesjo I just double checked and unfortunately it doesn't. I'm sure you don't want to be stuck on 19.0.3 indefinitely. Maybe a proper fix would be to trigger a resize event manually? It seems like they may have changed the algo to rely more on resize events. It does self correct if you resize the window.

image
image

@johannesjo
Copy link
Owner

This sounds like something they should fix on their end, though I am not sure how one would formulate the text for an issue in a way that it gets any attention.

I think triggering a resize event manually and only doing so if animations are disabled might be the best course of action. I am hesitant to merge this downgrade, since I can't rule out that it will introduce some problems with other components.

Could you possibly provide a PR for that?

@palmerusaf
Copy link
Contributor Author

@johannesjo I agree. I've considered submitting an issue with a minimal reproducible example over at the component library. As far as another PR, I can give it a shot but it may take me a while, as I'm still unfamiliar with angular and I'm taking a lot of classes at the moment.

@johannesjo
Copy link
Owner

That would be great! Let me know if you have any questions!

@johannesjo johannesjo closed this Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants