feat: add direction option for RTL menu support - #797
Conversation
Adds a `direction` option (default 'ltr') that, when set to 'rtl', applies a `context-menu-rtl` class to the menu and its sub-menus. CSS mirrors text alignment, icon placement and the submenu arrow, and the submenu positioning logic flips so sub-menus open to the left of their parent item instead of the right. Closes #742
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3655bdfd16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isRtl) { | ||
| if (left < $win.scrollLeft()) { | ||
| // doesn't fit to the left of the item, flip to the right | ||
| left = itemOffset.left + this.outerWidth() - 5; | ||
| } |
There was a problem hiding this comment.
When a detached RTL submenu cannot fit to the left of an opener near the viewport's left edge, this branch flips it to the right but never checks whether left + menuWidth now exceeds the viewport's right edge. If neither side has enough room, the submenu therefore remains partially off-screen instead of receiving the edge-clamping behavior described here; clamp the flipped result against the right boundary before applying the existing left clamp.
Useful? React with 👍 / 👎.
Addresses Codex review feedback on PR #797: when a detached RTL submenu didn't fit to the left of its opener and flipped to the right, it wasn't checked against the right viewport edge (only the existing left-edge clamp applied, which was the wrong side to guard after flipping right).
|
Addressed the Codex review feedback in 58794b2:
Verified with |
Resolves conflicts in src/jquery.contextMenu.js between this branch's RTL submenu-open-side flip and #798's overflow detach/cap-and-scroll logic (also touching positionSubmenu()). Combined so that: - the isRtl check picks which side (left/right) a sub-menu opens on and which side it flips back to if it doesn't fit there, - the overflow-detection/detach/cap-and-scroll logic (and its preciseOuterHeight()-based measurements) run independently of that side choice, since a sub-menu can be too tall regardless of which side it opens on. Both PRs' test coverage (test/unit/direction-rtl.test.js and test/specs/overflow-submenu-only.js) is preserved.
1 participant
Closes #742
Summary
Adds a
directionoption to$.contextMenu({...})(default'ltr'). When set to'rtl':context-menu-rtlclass is added to the root menu and every sub-menu (including detached sub-menus that get moved to<body>when their parent menu is scrollable, see Overflow / visibility issue if contextmenu longer than screen height #775, so they don't lose the styling when they're no longer a DOM descendant of the root).direction: rtl; text-align: right), icon placement (icon moves to the right edge instead of the left, covering both the icon-font and FontAwesome-5 code paths), checkbox/radio spacing, and the submenu-arrow triangle (now points left and sits on the left edge instead of the right).positionSubmenuinsrc/jquery.contextMenu.js) flips the jQuery UI.position()my/atarguments (and the equivalent fallback/detached-submenu math) so sub-menus open to the left of their parent item instead of the right, with the sameflipfit/edge-clamping behavior as the existing LTR code so it still flips back to the right if there isn't enough room on the left.Scope notes
This covers the two things asked for in #742 (menu item text direction/alignment, and RTL support in general): a CSS hook for styling and a real flip of which side sub-menus open on. Root-menu placement itself already has viewport collision handling (
fit) that isn't direction-specific, so it needed no changes.Files changed
src/jquery.contextMenu.js– newdirectiondefault option, class applied at menu-creation time,positionSubmenuflip for both the jQuery UI code path and the plain-JS/detached-submenu fallback paths.src/sass/jquery.contextMenu.scss–.context-menu-rtlrules mirroring text alignment, icon placement, submenu arrow and initial submenu position.documentation/docs.md– documents the newdirectionoption with an example.test/unit/direction-rtl.test.js– new QUnit regression tests: default menu has no rtl class,direction: 'rtl'adds the class to both the root menu and its sub-menus, and a sub-menu opens to the left of its parent item inrtlmode vs. to the right in the defaultltrmode.Test plan
npm run test-unit(karma + qunit, ChromeHeadless) – 34/34 passing, including the 3 new RTL tests.npm run test:fixtures && npx playwright test– 15/15 passing, no regressions.npx sass) to confirm it has no syntax errors.dist/was intentionally left untouched per this repo's convention of rebuilding it at publish time.