Fix Our Events Card Glitch After Toggling and Resizing Window

[tony1ee]

Fixes #6775

What changes did you make?

1. (7e94b92) changed where eventListener is registered from .flex-page-card' to window
2. (4262f25) added a condition before handleScreenResize() collapse cards to check if they have been manually unfolded, and if so, leave them unfolded

Why did you make the changes (we will use this info to test)?

• change No.1 was made to address the main issue in #6775, making the eventListener properly registered (with the current way of registration it won't be triggered) so it can function as intended to reset the cards after resizing
• change No.2 was made to address a issue that existed in the code before but was only exposed after change No.1 is made now that the function can be triggered: handleScreenResize() will reset (collapse) all cards every time the window size is #bp-below-tablet after resize. The problem is it cannot tell whether the previous state before resize event is also #bp-below-tablet. If the resize happens from a #bp-below-tablet width to another #bp-below-tablet width, the expected behavior is to leave the card as is, rather than resetting/collapsing it. By adding a condition to check if card was manually unfolded (by checking existence of active in class list), the unfolded cards will not be unexpectedly collapsed, therefore fixing the problem.

Screen Recordings of Proposed Changes Of The Website

Visuals before change 1 is applied

notice how the card will be empty after resizing from width 750 to width 800

Visuals after change 1 is applied

notice how the card content remains after resizing from width 750 to width 800

Visuals after change 1 is applied but before change 2 is applied

notice the card collapse unexpectedly while adjusting the width (<=767 the entire duration)

Visuals after both change 1 and change 2 are applied

expected behavior: the unfolded card remains unfolded while adjusting the width (<=767 the entire duration)

[gaylem]

Hey @tony1ee,

Amazing work on this issue! It functions as expected for me locally. You also set up your branch and linked out to the issue correctly. Well done!

One observation that probably doesn't matter:

• I noticed that the drop-downs toggle shut when the when the screen is reduced from > 767 to < 767. This might cause user frustration if someone is reading the content and tries to shrink their screen. Could disorient them a bit.
• It's such an edge case, though, and fixing that seems like it would be way more complicated than its worth in this instance. Besides, how would we know someone is reading the content? I think addressing this would require too much of a redesign and falls outside of the scope of this issue. Just thought I'd mention it in case anyone else felt the same way.

Otherwise, I only have nit-picky change requests:

• This is optional, but if you'd like to clean up the code you modified to follow the (AirBnB style guide) that would look pretty. I haven't heard if we follow a specific style guide but this is the one I see most commonly used.
• Given this feature is a bit finicky, would you mind adding some more comments to the event listener for future devs so they understand why it's built this way? JSDoc comments would also be rad, but you can do it however you want.

Thanks, dude! 👍

[gaylem]

Note for other reviewers: The Add Pull Request Instructions check is failing because of #6778. The changes were reverted, but its still not working for this issue. Here's how I set up my branch locally so you don't have to figure it out:

git checkout -b tony1ee-fix-events-card-glitch-6775
git pull https://github.com/tony1ee/website.git fix-events-card-glitch-6775

Note for merge team: I'm guessing we can ignore that failed check and just squash and merge when this gets two approvals. Let me know if you disagree.

[Thinking-Panda]

@tony1ee , nice work with the issue. Code changes give expected result. Website works well on my local machine. I approve!

[tony1ee]

@gaylem Thanks for your thorough review.

You were correct to point out the behavior of the card when first switched to/loaded mobile view. I am not sure what is the expected behavior in terms of default card display. I would consider it out of scope of this issue and suggest doing an issue involving UI/UX to address it further.

As for your 2nd concern, I pushed another commit with comments and some format adjustments. I tried my best to balance between maintainability and keeping the changes within issue scope. Please let me know your thoughts!

[gaylem]

@tony1ee I agree with your comments and the changes look great, approved!