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(Drawer,Flyout): alignment #2910

Merged
merged 1 commit into from
Aug 5, 2024
Merged

Conversation

jrood
Copy link
Contributor

@jrood jrood commented Jul 12, 2024

Overview

This PR fixes a layout issue where flyout content had been rendering slightly shifted to the left in certain cases. This is a complex issue having to do with the fact that overflow="hidden" in the Drawer component causes reflow during the animation, specifically when it occurs within the fixed Flyout component.

The Drawer component currently animates the width of a parent wrapper, while holding content in a child wrapper. The main fix is to use overflow="clip" (which does not cause reflow) rather than overflow="hidden" on the parent wrapper, and to move overflowY scrolling to the child wrapper.

Example of how this fixes the unintended layout shift in mobile catalog menus:

before after
Screenshot 2024-07-24 at 11 03 41 AM Screenshot 2024-07-24 at 11 04 05 AM
Screenshot 2024-07-24 at 2 21 39 PM Screenshot 2024-07-24 at 2 21 45 PM

Note, in before, the issue is not a 2px right-margin, but rather that the entire content is shifted left so that it would be slightly cutoff if there was not already left margin.

An additional fix included here is to ensure content is aligned opposite of the openFrom side. During an animation, content coming from the left should be aligned to the right (as it moves with the parent wrapper's moving right edge). Conversely, content from the right should be aligned to the left.

(I apologize in advance for any dizziness invoked by the ternary expression: 'left' ? 'right' : 'left'.)

A resize observer was added to popover to ensure that the tooltip for the close button does not get lost in the animation. This means that the tooltip will (correctly) show in some places where it doesn't currently in prod.

Screenshot 2024-08-01 at 5 25 12 PMScreenshot 2024-08-01 at 5 24 50 PM

However there is a plan to remove this automatic focus soon.

Fix alignment of drawer and flyout.

PR Checklist

  • Related to designs:
  • Related to JIRA ticket: DOT-386
  • I have run this code to verify it works
  • This PR includes unit tests for the code change
  • This PR includes testing instructions tests for the code change
  • The alpha package of this PR is passing end-to-end tests in all relevant Codecademy repositories

Testing Instructions

Places to test:

Code and Output buttons in codebyte

My home menu on mobile + logged in dashboard

LE flyouts Ask the AI Learning Assistant Get Unstuck Tools Syllabus
LE preview

Admin: Add new coupon flyout on coupons page

Catalog Menu on mobile catalog home

Filters (scroll down) on catalog home

Article Categories menu on mobile articles

Topics on mobile docs

PR Links and Envs

Mono PR

Monolith PR

@jrood jrood changed the title Fix drawer and flyout alignment issue fix(Drawer,Flyout): alignment Jul 12, 2024
@jrood jrood force-pushed the jrood.drawer-flyout-align branch from 238faf2 to cfb0b4c Compare July 15, 2024 15:31
@jrood jrood force-pushed the jrood.drawer-flyout-align branch 5 times, most recently from ef92892 to ce5a845 Compare July 25, 2024 15:09
@jrood jrood force-pushed the jrood.drawer-flyout-align branch from ce5a845 to b51b020 Compare July 30, 2024 16:38
@jrood jrood marked this pull request as ready for review August 1, 2024 17:12
@jrood jrood requested a review from a team as a code owner August 1, 2024 17:12
@jrood jrood force-pushed the jrood.drawer-flyout-align branch from b51b020 to 5ce4d33 Compare August 2, 2024 18:08
@codecademydev
Copy link
Collaborator

📬Published Alpha Packages:

@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]

@codecademydev
Copy link
Collaborator

🚀 Styleguide deploy preview ready!

https://66ad22b5882592a1436a870a--gamut-preview.netlify.app

Deploy Logs

Copy link
Contributor

@dreamwasp dreamwasp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks awesome! this is tricky little component, thanks for refining it!

@jrood jrood added the Ship It :shipit: Automerge this PR when possible label Aug 5, 2024
@codecademydev codecademydev merged commit 3a931d9 into main Aug 5, 2024
19 of 20 checks passed
@codecademydev codecademydev removed the Ship It :shipit: Automerge this PR when possible label Aug 5, 2024
@codecademydev codecademydev deleted the jrood.drawer-flyout-align branch August 5, 2024 19:57
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.

4 participants