Skip to content

fix(PopoverContainer): working in RTL #3333

Merged
LinKCoding merged 10 commits intocass-gmt-1601from
kl-gmt-1598-popovercontainer-alignment
Apr 29, 2026
Merged

fix(PopoverContainer): working in RTL #3333
LinKCoding merged 10 commits intocass-gmt-1601from
kl-gmt-1598-popovercontainer-alignment

Conversation

@LinKCoding
Copy link
Copy Markdown
Contributor

@LinKCoding LinKCoding commented Apr 24, 2026

Overview

Updates PopoverContainer to revert back to use physical properties in lieu of logical properties.
The major constraint was how logical properties flip (or double-flip) and do not work with the math involved in positioning a PopoverContainer.

PR Checklist

  • Related to designs:
  • Related to JIRA ticket: GMT-1598
  • Version plan added/updated (or not needed)
  • 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

Don't make me tap the sign.

  1. Go to PopoverContainer
  2. Check that inverting axis works as expected for inline/non-inline variants
  3. Repeat for LTR/RTL and logical and physical (tho RTL can be omitted for physical)
  4. Check Popover and see that stories still look fine, check against beak positioning and alignment
  5. Check InfoTip and see that stories still look fine, check against alignment and positioning
  6. ...
  7. Finish and do a celebratory dance

PR Links and Envs

Repository PR Link
Mono Mono PR

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Apr 24, 2026

View your CI Pipeline Execution ↗ for commit d76da5a


☁️ Nx Cloud last updated this comment at 2026-04-29 13:57:33 UTC

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 95.91837% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.31%. Comparing base (b5a282c) to head (d76da5a).
⚠️ Report is 1 commits behind head on cass-gmt-1601.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...es/gamut/src/PopoverContainer/PopoverContainer.tsx 94.73% 1 Missing ⚠️
packages/gamut/src/PopoverContainer/utils.ts 96.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           cass-gmt-1601    #3333      +/-   ##
=================================================
+ Coverage          89.25%   89.31%   +0.06%     
=================================================
  Files                251      251              
  Lines               4719     4755      +36     
  Branches            1570     1619      +49     
=================================================
+ Hits                4212     4247      +35     
- Misses               499      500       +1     
  Partials               8        8              
Flag Coverage Δ
pull-request 89.31% <95.91%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LinKCoding LinKCoding force-pushed the kl-gmt-1598-popovercontainer-alignment branch from c91d7f0 to a1befa9 Compare April 28, 2026 19:57
@LinKCoding LinKCoding force-pushed the kl-gmt-1598-popovercontainer-alignment branch from a1befa9 to a7779c6 Compare April 28, 2026 19:58
@LinKCoding LinKCoding marked this pull request as ready for review April 29, 2026 13:00
@LinKCoding LinKCoding requested a review from a team as a code owner April 29, 2026 13:00

const { dirNeutralStyles, styles: placementStyles } = popoverPosition;

/** Non-empty `placementStyles` merged into inline `style`; `dirNeutralStyles` layered last (see `getPosition`). */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is the tiniest nit ever but could you swap to the same comment style as the one above?
/** Line one

  • two
    **/

Copy link
Copy Markdown
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.

looke great in SB, testing mono now

@codecademydev
Copy link
Copy Markdown
Collaborator

📬 Published Alpha Packages:

Package Version npm Diff
@codecademy/gamut 68.2.3-alpha.eb3f22.0 npm diff
@codecademy/gamut-icons 9.57.3-alpha.eb3f22.0 npm diff
@codecademy/gamut-illustrations 0.58.10-alpha.eb3f22.0 npm diff
@codecademy/gamut-kit 0.6.593-alpha.eb3f22.0 npm diff
@codecademy/gamut-patterns 0.10.29-alpha.eb3f22.0 npm diff
@codecademy/gamut-styles 17.13.2-alpha.eb3f22.0 npm diff
@codecademy/gamut-tests 5.3.4-alpha.eb3f22.0 npm diff
@codecademy/variance 0.26.2-alpha.eb3f22.0 npm diff
eslint-plugin-gamut 2.4.4-alpha.eb3f22.0 npm diff

@github-actions
Copy link
Copy Markdown
Contributor

@LinKCoding LinKCoding merged commit 3353c5e into cass-gmt-1601 Apr 29, 2026
13 checks passed
@LinKCoding LinKCoding deleted the kl-gmt-1598-popovercontainer-alignment branch April 29, 2026 14:05
dreamwasp added a commit that referenced this pull request Apr 29, 2026
* chore(GamutProvider): useLogicalProperties hook

* feat(gamut-styles): add directionIsRtl utility

Move DOM direction resolution (including JSDOM fallback) into
@codecademy/gamut-styles utilities and export alongside useLogicalProperties.

* fix(InfoTip): update rtl styles (#3319)

* bug(InfoTip): update rtl styles

Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>

* fix(InfoTip): update rtl styles (#3319)

* bug(InfoTip): update rtl styles

Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>

* Update packages/styleguide/src/lib/Meta/Logical and physical CSS properties.mdx

Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>


* fix(Pagination): Previous and Next Buttons to respect RTL (#3316)

* chore: Update SelectDropdown Multi select to use logical properties (#3331)

* fix: popover alignment for rtl / useLogicalProperties (#3326)

* fix: popover, need to test invertAxis

* fix: Address followups for ToolTip bugs RTL (#3330)

* fix(Popover): addressing left/right beak (#3332)


Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>

* fix(PopoverContainer): working in RTL  (#3333)

* using physical properties for PopoverContainer positioning, mirroring is manual
* fix tests

---------

Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>
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.

3 participants