Skip to content

fix(popupreply): Adjust positioning for IE11#609

Merged
mergify[bot] merged 4 commits intobox:masterfrom
ConradJChan:ie11-popupreply
Oct 1, 2020
Merged

fix(popupreply): Adjust positioning for IE11#609
mergify[bot] merged 4 commits intobox:masterfrom
ConradJChan:ie11-popupreply

Conversation

@ConradJChan
Copy link
Contributor

Due to a previous fix (#540), in IE11 the PopupReply doesn't have an elevated stacking context and as a result will appear behind the subsequent page if an annotation is created at the bottom of a page of a document.

This PR makes an IE11 specific workaround by defaulting the positioning of the PopupReply to the top of the element rather than the bottom. The PopupList of collaborators is also shortened to avoid getting clipped by the subsequent page.

This does not fully solve the problem as there is a edge case that still fails when an annotation is created at the bottom of the page of a document, but the scroll position for the page is such that it is also bounded by the top of the viewport -- then the PopupReply appears below (to avoid the top of the viewport) but then still gets cut off by the subsequent page.

Non IE11:
Screen Shot 2020-09-30 at 1 17 57 PM

IE11 (before):
Screen Shot 2020-09-30 at 1 25 34 PM

IE11 (after):
Screen Shot 2020-09-30 at 1 18 19 PM

@ConradJChan ConradJChan requested a review from a team as a code owner September 30, 2020 20:36
const prevScale = usePrevious(scale);

// Keep the options reference the same between renders
if (!popupOptions.current) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be in a React.useEffect call that only occurs on mount?

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 idea I'll try that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think that works because the hook runs after render and as a result the placement gets misplaced a bit

jstoffan
jstoffan previously approved these changes Oct 1, 2020
@mergify mergify bot merged commit 032cbe7 into box:master Oct 1, 2020
@ConradJChan ConradJChan deleted the ie11-popupreply branch October 1, 2020 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants