sudara / alonetone

A free, open source, non-commercial home for musicians and their music
https://alonetone.com
MIT License
349 stars 89 forks source link

Rewrite track reveal animation to GSAP, to fix window resizing bugs #1103

Closed ofsound closed 3 years ago

ofsound commented 3 years ago

Fixes #1105

TODO

_Resizing the browser window both larger and smaller after a track reveal had been opened and closed exposed an issue with making only a one time height calculation of the reveal. Partial bits of the trackreveal could be seen peaking out when they should have remained hidden

This solution will hide the track reveal completely right after its closing animation.

Upon re-opening it will re-calculate a position exactly off screen to start its animation back to margin-top: 0px

I added an extra class to let the CSS know about the special asset case of a track with the favorited by section.

The 'open' class is still used to set other CSS properties, but is no longer a trigger for a CSS transition.

I slightly re-wrote the currentlyOpen section of the JS, before there was a conditional in the openDetails which seemed unnecessary.

I attempted to have a single GSAP timeline with labels in the initialize(), but it wasn't calculating the offsetHeight correctly, so I kept a single GSAP timeline to avoid multiple tween directions, but have the open and close methods clearing that timeline and applying their transitions.

The homepage spec is breaking, but it doesn't seem too serious. Maybe elements are just offscreen in a slightly different way or order. We'll see....

"Ready For Review" checklist

These checklists are to help ensure the code review basics are covered. Consider removing to reduce noise.

Before code review and after additional commits during review.

sudara commented 3 years ago

Looks like there's a css merge conflict....

sudara commented 3 years ago

Dug deep here. I'm not 100% sure why the tests are only failing on the branch actually. They fail locally when the elements are out of view. Then capybara has problems being told to click on something unless it's explicitly passed visible: :all which will let it click on something below the fold or otherwise hidden.

Added some details about how to run those feature tests locally so you can try it out next time. That actually had broken though, so I got that happy again, cleaned up the config, etc. 😰

sudara commented 3 years ago

Hmm, tests still fail on CI, but they are passing locally. :(

sudara commented 3 years ago

This particular error doesn't seem to show up until yesterday.

Failure/Error: find('.play_button_container a').click # pause

I believe it might be related to me manually setting the screen size that CI is running at. I do get this problem locally with the seeds. Ideally we'd bypass fixing it because it'll move to a new paradigm after #1087

sudara commented 3 years ago

Closing in favor of #1108 — nice work on the js stuff!