fix(popupreply): Move PopupReply into new PopupManager layer by ConradJChan · Pull Request #603 · box/box-annotations

ConradJChan · 2020-09-24T06:45:03Z

Issue
With the way that PopupReply was rendered in the same layer as the annotation-specific layer (region, highlight), the popup in some browsers (ie11) would be beneath other drawn annotation elements, depending on what order the annotation layers were configured.

Overview
Creates a new PopupManager that renders the PopupReply for the staged annotation, regardless of annotation type so that it can always be the top layer.

Before

After

TODO

unit tests
cross-browser testing

ChenCodes · 2020-09-24T17:55:05Z

Could you add a gif of the experience?

src/components/Popups/PopupReply.tsx

src/highlight/HighlightAnnotations.tsx

src/popup/PopupContainer.tsx

ConradJChan · 2020-09-24T18:27:32Z

@ChenCodes Updated with screenshots -- before the highlight would appear above the create region PopupReply

src/store/creator/actions.ts

src/highlight/HighlightAnnotations.tsx

src/common/__tests__/useWindowSize-test.tsx

src/store/creator/actions.ts

src/region/__tests__/RegionAnnotations-test.tsx

src/region/RegionAnnotations.tsx

src/popup/__tests__/PopupLayer-test.tsx

src/popup/__mocks__/PopupLayer.tsx

src/document/__tests__/DocumentAnnotator-test.ts

jstoffan · 2020-09-25T18:02:34Z

src/popup/PopupLayer.tsx

+        status,
+    } = props;
+
+    const [reference, setReference] = React.useState<PopupReference | null>(null);


Is the extra state here needed or can we derive and pass the virtualElement directly in render?

I had originally done just that, but I think the fact that virtualElement was a new object each render caused some wonky behavior in IE11

src/region/RegionAnnotations.tsx

ConradJChan force-pushed the move-popupreply branch from b132dc6 to 2c9fc1c Compare September 24, 2020 06:46

ChenCodes reviewed Sep 24, 2020

View reviewed changes

src/components/Popups/PopupReply.tsx Show resolved Hide resolved

ChenCodes reviewed Sep 24, 2020

View reviewed changes

src/highlight/HighlightAnnotations.tsx Outdated Show resolved Hide resolved

ChenCodes reviewed Sep 24, 2020

View reviewed changes

src/popup/PopupContainer.tsx Show resolved Hide resolved

mingzexiao6 reviewed Sep 24, 2020

View reviewed changes

src/store/creator/actions.ts Outdated Show resolved Hide resolved

src/highlight/HighlightAnnotations.tsx Outdated Show resolved Hide resolved

Conrad Chan added 2 commits September 24, 2020 23:44

fix(popupreply): Move PopupReply into new PopupManager layer

69746a1

chore: pr comments and unit tests

9dc2be9

ConradJChan force-pushed the move-popupreply branch from 2c9fc1c to 9dc2be9 Compare September 25, 2020 06:49

ConradJChan marked this pull request as ready for review September 25, 2020 06:50

ConradJChan requested a review from a team as a code owner September 25, 2020 06:50