Skip to content

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

Merged
LinKCoding merged 3 commits intocass-gmt-1601from
kl-gmt-1611-multi-select
Apr 22, 2026
Merged

chore: Update SelectDropdown Multi select to use logical properties#3331
LinKCoding merged 3 commits intocass-gmt-1601from
kl-gmt-1611-multi-select

Conversation

@LinKCoding
Copy link
Copy Markdown
Contributor

@LinKCoding LinKCoding commented Apr 22, 2026 •
edited
Loading

Overview

PR Checklist

  • Related to designs:
  • Related to JIRA ticket: GMT-1611
  • 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 SelectDropdown story and the Multi-select section
  2. Check that Physical LTR looks and works as intended
  3. Check that Logical LTR and RTL looks and works as intended
  4. ...
  5. Finish and do a celebratory dance

PR Links and Envs

Repository PR Link
Mono Mono PR

@LinKCoding LinKCoding changed the base branch from main to cass-gmt-1601 April 22, 2026 15:17
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Apr 22, 2026 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 27f8062

☁️ Nx Cloud last updated this comment at 2026-04-22 15:30:15 UTC

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Apr 22, 2026

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.

View your CI Pipeline Execution ↗ for commit acf9499

Command Status Duration Result
nx run-many --target=build --all ❌ Failed 42s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-22 15:18:07 UTC

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (cass-gmt-1601@6efbb0b). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on cass-gmt-1601.
✅ All tests successful. No failed tests found.

Additional details and impacted files 
@@               Coverage Diff                @@
##             cass-gmt-1601    #3331   +/-   ##
================================================
  Coverage                 ?   89.19%           
================================================
  Files                    ?      250           
  Lines                    ?     4702           
  Branches                 ?     1590           
================================================
  Hits                     ?     4194           
  Misses                   ?      500           
  Partials                 ?        8
Flag Coverage Δ
pull-request 89.19% <100.00%> (?)

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.

@codecademydev
Copy link
Copy Markdown
Collaborator

codecademydev commented Apr 22, 2026

📬 Published Alpha Packages:

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

🚀 Styleguide deploy preview ready!

Preview URL: https://69e8ea543b6840a925a562e3--gamut-preview.netlify.app
Deploy Logs: https://app.netlify.com/projects/gamut-preview/deploys/69e8ea543b6840a925a562e3

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

🚀 Styleguide deploy preview ready!

Preview URL: https://69e8ea54d14c773ddfba8384--gamut-preview.netlify.app
Deploy Logs: https://app.netlify.com/projects/gamut-preview/deploys/69e8ea54d14c773ddfba8384

LinKCoding
LinKCoding commented Apr 22, 2026
View reviewed changes
Comment thread packages/gamut/src/Form/SelectDropdown/styles.ts
Comment on lines +188 to +191
borderEndEndRadius: 0,
borderEndStartRadius: theme.borderRadii.md,
borderStartEndRadius: 0,
borderStartStartRadius: theme.borderRadii.md,
Copy link
Copy Markdown
Contributor Author

@LinKCoding LinKCoding Apr 22, 2026

Choose a reason for hiding this comment

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

Opting to hardcode logical properties here since:

  1. it's a styles.ts file so using the useLogicalProperties just adds some bloat that might require clean up
  2. Rendering is still the same as when physical properties are used

but long story short, probably not worth jumping through hoops to ensure that users have the option of physical vs logical when logical properties do the same thing in LTR but also support RTL

@LinKCoding LinKCoding marked this pull request as ready for review April 22, 2026 18:46
@LinKCoding LinKCoding requested a review from a team as a code owner April 22, 2026 18:46
aresnik11
aresnik11 approved these changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@aresnik11 aresnik11 left a comment

Choose a reason for hiding this comment

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

LGTM!

@LinKCoding LinKCoding merged commit 2bb07ee into cass-gmt-1601 Apr 22, 2026
14 checks passed
@LinKCoding LinKCoding deleted the kl-gmt-1611-multi-select branch April 22, 2026 18:59
dreamwasp added a commit that referenced this pull request Apr 29, 2026
@dreamwasp @LinKCoding
chore(GamutProvider, Popover, PopoverContainer): RTL fixes (#3306)
f8c7d17 
* 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

Reviewers

@aresnik11 aresnik11 aresnik11 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LinKCoding @codecademydev @aresnik11