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

Add horizontalAlignment to Menu.open() options #732

Merged
merged 13 commits into from
Dec 19, 2024

Conversation

andrewfulton9
Copy link
Contributor

Closes #709

Adds an "align" option to Menu.IOpenOptions that can be set to "left" or "right" and adjusts the menu position so the top left or right corner is matched to the given x and y coordinates based on the given align value. It defaults to left or right depending on the document dir attribute to account for the given language direction.

@krassowski
Copy link
Member

Thank you @andrewfulton9!

  1. To refresh review/api/widgets.api.md you can run yarn run api locally
  2. The tests are failing on Mac/Safari due to rounding issues:
 ❌ @lumino/widgets > Menu > #open() > should accept align menu flags
      AssertionError: expected 'translate(205.53125px, 300px)' to equal 'translate(205.531px, 300px)'
      + expected - actual
      
      -translate(205.53125px, 300px)
      +translate(205.531px, 300px)
      
      at tests/lib/bundle.test.js:39356:59

 ❌ @lumino/widgets > Menu > #open() > align should default to right if language direction is rtl
      AssertionError: expected 'translate(205.53125px, 300px)' to equal 'translate(205.531px, 300px)'
      + expected - actual
      
      -translate(205.53125px, 300px)
      +translate(205.531px, 300px)   

Maybe we could use a regular expression pattern here, optionally allowing the number to have more decimal digits?

@andrewfulton9
Copy link
Contributor Author

@krassowski, I updated the tests so only the whole number of the x value is checked. It should be good to go now!

*
* The default is `'left'` unless the document `dir` attribute is `'rtl'`
*/
align?: 'left' | 'right';
Copy link
Member

Choose a reason for hiding this comment

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

To make this more future-proof, should we call it horizontalAlignment instead? So that we can add verticalAlignment if we need it later?

Otherwise it looks great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I just updated that.

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you @andrewfulton9!

@krassowski krassowski changed the title Align menu Add horizontalAlignment to Menu.open() options Dec 19, 2024
@krassowski krassowski merged commit 53f937e into jupyterlab:main Dec 19, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Menu.open() should have an option to align x and y to the top right of the menu.
2 participants