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

feat: add FullWindowOverlay component for Fabric #1495

Merged
merged 12 commits into from
Jun 29, 2022
Merged

feat: add FullWindowOverlay component for Fabric #1495

merged 12 commits into from
Jun 29, 2022

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Jun 28, 2022 •
edited
Loading

Description

Added FullWindowOverlay component support for Fabric.

Changes

  • Implemented new init method & extracted common initialisation code to separate method.
  • Adjusted some types that are paper specific (RCTTouchHandler -> RCTSurfaceTouchHandler)
  • Added Fabric specific boilerplate
  • Added some Fabric specific logic to solve the aroused issues with view hierarchy & view recycling. See comments added in this commit.

Test code and steps to reproduce

  • Test1096 in FabricTestExample

Checklist

  • Included code example that can be used to test this change
  • Updated TS types
  • Ensured that CI passes

@kkafar kkafar force-pushed the @kkafar/fabric-full-window-overlay branch from 30269fc to e8f6163 Compare June 28, 2022 13:09
@kkafar
chore: add prepareForRecycle method
1c5c4d9 
However more extensive logic may be required for Fabric as it seems to
me, that _container wont be initialized for the second time
@kkafar kkafar force-pushed the @kkafar/fabric-full-window-overlay branch from 7ab9905 to 1c5c4d9 Compare June 29, 2022 08:48
@kkafar
chore: move reactSetFrame method to paper specific section
c2442c4
@kkafar
chore: mount child component view & update layout metrics
383a075
@kkafar
fix: add maybeShow method.
b559650 
When the component unmounts we remove it from window's children,
so when the component gets recycled we need to add it back.

Also see comments added in the code.
@kkafar kkafar marked this pull request as ready for review June 29, 2022 12:14
@kkafar kkafar requested a review from WoLewicki June 29, 2022 12:18
WoLewicki
WoLewicki approved these changes Jun 29, 2022
View reviewed changes
Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@kkafar kkafar merged commit 16289ce into main Jun 29, 2022
@kkafar kkafar deleted the @kkafar/fabric-full-window-overlay branch June 29, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WoLewicki WoLewicki WoLewicki 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.
2 participants
@kkafar @WoLewicki