From b3c120dde7b407a31849a7a88c4ee960c6f518d2 Mon Sep 17 00:00:00 2001
From: Seth Foster <seth@opentrons.com>
Date: Fri, 17 May 2024 10:16:27 -0400
Subject: [PATCH] fix(app): allow navbar during LegacyModals on the modal
 portal (#15166)

In theory, the intent both from a programming and a design aspect of
having the "modal portal root" - the portal component that lives inside
the desktop app's route-component render node - was that modals
registered to the modal portal instead of the top portal would occlude
page content (i.e. the Devices page, a protocols page, whichever one
that modal said it wanted) but _not_ occlude or inhibit interaction with
the navbar or the breadcrumbs.

However, because the Overlay component behind LegacyModal (which is the
thing that renders the translucent occlusion of background content, and
the thing that intercepts interaction) had `position: fixed`, it would
always be positioned as-if its parent were the viewport, meaning that it
would always occlude interaction with the entire app, even while
centering the actual modal content on the page content. This is wrong.

The solution is for Overlay to be `position: absolute`, which has most
of the same semantics (also creates a z-order stack, allows `left:`,
`right:`, `top:`, `bottom:`, etc) but has its positioning parent be its
DOM parent, which in this case is the modal portal element, which means
that
- When a `LegacyModal` is instantiated via `createPortal` to the "top
level portal root", it will occlude and intercept interaction to
everything in the viewport (also previous behavior, correct)
- When a `LegacyModal` is instantiated via `createPortal` to the "modal
portal root", it will occlude and intercept interaction to the page
content but not the navbar or breadcrumbs (new behavior, correct)
- When a `LegacyModal` is instantiated via a normal component render
tree, it will occlude and intercept interaction to whatever is below it
in the component tree (new behavior, correct - previously this would
also intercept and occlude the entire viewport)

This is a lot of words for a small css change, but it affects a whole
lot of things.

## Review and Testing Requests
- There should be no change to modals that use the top root, let's smoke
test this and check; that should include app update notifications and
estop modals
- We should smoke test the following modals, which use the modal portal
and a `LegacyModal`:
    - [x] Confirm cancelling a run
    - [x] Robot update notifications
---
 app/src/molecules/LegacyModal/LegacyModalShell.tsx            | 3 +--
 .../Devices/RobotSettings/UpdateBuildroot/ViewUpdateModal.tsx | 4 ++--
 app/src/organisms/RunDetails/ConfirmCancelModal.tsx           | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/app/src/molecules/LegacyModal/LegacyModalShell.tsx b/app/src/molecules/LegacyModal/LegacyModalShell.tsx
index d18adc211ac..431b607c815 100644
--- a/app/src/molecules/LegacyModal/LegacyModalShell.tsx
+++ b/app/src/molecules/LegacyModal/LegacyModalShell.tsx
@@ -2,7 +2,6 @@ import * as React from 'react'
 import styled from 'styled-components'
 import {
   COLORS,
-  POSITION_FIXED,
   POSITION_ABSOLUTE,
   ALIGN_CENTER,
   JUSTIFY_CENTER,
@@ -76,7 +75,7 @@ export function LegacyModalShell(props: LegacyModalShellProps): JSX.Element {
 }
 
 const Overlay = styled.div`
-  position: ${POSITION_FIXED};
+  position: ${POSITION_ABSOLUTE};
   left: 0;
   right: 0;
   top: 0;
diff --git a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/ViewUpdateModal.tsx b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/ViewUpdateModal.tsx
index ea8435ab8bb..37e4177db0d 100644
--- a/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/ViewUpdateModal.tsx
+++ b/app/src/organisms/Devices/RobotSettings/UpdateBuildroot/ViewUpdateModal.tsx
@@ -10,7 +10,7 @@ import {
   getRobotUpdateAvailable,
 } from '../../../../redux/robot-update'
 import { getAvailableShellUpdate } from '../../../../redux/shell'
-import { getModalPortalEl } from '../../../../App/portal'
+import { getTopPortalEl } from '../../../../App/portal'
 import { UpdateAppModal } from '../../../../organisms/UpdateAppModal'
 import { MigrationWarningModal } from './MigrationWarningModal'
 import { UpdateRobotModal } from './UpdateRobotModal'
@@ -60,7 +60,7 @@ export function ViewUpdateModal(
   if (availableAppUpdateVersion && showAppUpdateModal)
     return createPortal(
       <UpdateAppModal closeModal={() => setShowAppUpdateModal(false)} />,
-      getModalPortalEl()
+      getTopPortalEl()
     )
 
   if (showMigrationWarning) {
diff --git a/app/src/organisms/RunDetails/ConfirmCancelModal.tsx b/app/src/organisms/RunDetails/ConfirmCancelModal.tsx
index 2e1b6b5238f..ba1ec7e738f 100644
--- a/app/src/organisms/RunDetails/ConfirmCancelModal.tsx
+++ b/app/src/organisms/RunDetails/ConfirmCancelModal.tsx
@@ -20,7 +20,7 @@ import {
 } from '@opentrons/api-client'
 import { useStopRunMutation } from '@opentrons/react-api-client'
 
-import { getModalPortalEl } from '../../App/portal'
+import { getTopPortalEl } from '../../App/portal'
 import { LegacyModal } from '../../molecules/LegacyModal'
 import { useTrackProtocolRunEvent, useIsFlex } from '../Devices/hooks'
 import { useRunStatus } from '../RunTimeControl/hooks'
@@ -108,6 +108,6 @@ export function ConfirmCancelModal(
         </Flex>
       </Flex>
     </LegacyModal>,
-    getModalPortalEl()
+    getTopPortalEl()
   )
 }