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.

Sign up for GitHub

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

Jump to bottom

[popover2] fix(ContextMenu2): simpler target positioning logic #4740

Merged
merged 2 commits into from
May 27, 2021
Merged

[popover2] fix(ContextMenu2): simpler target positioning logic #4740

merged 2 commits into from
May 27, 2021

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented May 27, 2021 •
edited
Loading

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Wrap ContextMenu2's generated popover target in a <Portal> to make its implementation align more closely with the legacy ContextMenuTarget API.
    • This obviates the need for attaching our own container ref and querying its containing positioning block with Classes.FIXED_POSITIONING_CONTAINING_BLOCK, making the API simpler for users in many use cases
    • This fixes a bug where the offset calculation was wrong in cases where the context menu child scrolled around in its container and/or the developer was not able to attach Classes.FIXED_POSITIONING_CONTAINING_BLOCK to the most relevant DOM element (for example, when using third-party libraries)
  • Deprecate Classes.FIXED_POSITIONING_CONTAINING_BLOCK
  • ❌ BREAKING CHANGE: Removed ref from ContextMenu2ChildrenProps

Reviewers should focus on:

No regressions in ContextMenu2 and Tree examples in docs

Screenshot

image

image

@blueprint-bot
Copy link

[popover2] fix(ContextMenu2): simpler target positioning logic

Previews: documentation | landing | table

@adidahiya

This comment has been minimized.

Sign in to view
@adidahiya

This comment has been minimized.

Sign in to view
@blueprint-bot
Copy link

switch back to clientX/Y

Previews: documentation | landing | table

@adidahiya adidahiya merged commit a649ded into develop May 27, 2021
@adidahiya adidahiya deleted the ad/context-menu2-portal branch May 27, 2021 17:54
@adidahiya adidahiya mentioned this pull request Jun 9, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adidahiya @blueprint-bot