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

move dialog resizers into shadow dom #1836

Merged
merged 6 commits into from
Feb 8, 2024
Merged

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented Feb 8, 2024

Changes

This is a slight refactor of the Resizer component used in Dialog.

  • It is now rendered inside shadow DOM, and its styles have been updated to use CSS grid for positioning. As an added benefit of shadow DOM, I was able to use a plain <style> here (vs inline styles) without causing performance issues.
  • User-facing change: It now protrudes outside the edge of the dialog, as originally intended (before it was reversed in fix resizable dialog issues #1424 due to overflow, which was recently removed in remove overflow and inline transform from Dialog #1795).
    • Screenshot below visualizes the resizers (corners in hotpink, perpendiculars in wheat); the corner resizers appear on top using a higher z-index.
      image

Internal change: Extracted out ShadowTemplate into a shared component and renamed to ShadowRoot. Updated the logic inside to correctly handle isomorphic rendering (via declarative syntax). Also added a flushSync to immediately force a re-render (needed by createPortal); this ensures that any layout measurements are immediately reflected correctly. This is important because the Dialog resizing logic immediately measures the DOM inside a useLayoutEffect and was reporting incorrect measurements if the shadow root content was not present on first mount.

Testing

Updated affected unit tests. Since attachShadow is called inside queueMicrotask, I had to use fake timers.

Manually tested that resizing works correctly. Inspect the Resizable Dialog story and play with it in devtools to understand the new changes.

Docs

Added changeset for the one-user facing change.

@mayank99 mayank99 self-assigned this Feb 8, 2024
@mayank99 mayank99 force-pushed the mayank/resizer-shadowdom branch from 907202c to 122494f Compare February 8, 2024 00:38
@mayank99 mayank99 marked this pull request as ready for review February 8, 2024 00:47
@mayank99 mayank99 requested review from a team as code owners February 8, 2024 00:47
@mayank99 mayank99 requested review from r100-stack and AdamMeza-Bentley and removed request for a team February 8, 2024 00:47
@FlyersPh9
Copy link
Collaborator

I don't think I'm the best person to review the .tsx changes, but I will say the resize interaction does feel much nicer. 👍

@mayank99 mayank99 merged commit 6f93692 into main Feb 8, 2024
16 checks passed
@mayank99 mayank99 deleted the mayank/resizer-shadowdom branch February 8, 2024 17:37
@imodeljs-admin imodeljs-admin mentioned this pull request Feb 8, 2024
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