rusackas opened a new pull request, #42548:
URL: https://github.com/apache/superset/pull/42548

   ### SUMMARY
   Alternate fix for #42510, following up on the discussion in #42546. That PR 
bumps `UNSAVED_CHANGES_MODAL_Z_INDEX` from 1100 to 1300 to clear a specific 
case where the View SQL modal (dynamically z-indexed by Ant Design to around 
1200) rendered on top of the unsaved-changes dialog. The problem with bumping 
the constant is that 1200 isn't a fixed ceiling, it's whatever Ant Design's own 
auto z-index stacking computes for however many popups happen to be layered at 
that moment, so the same bug could resurface with a different number later.
   
   This drops `UNSAVED_CHANGES_MODAL_Z_INDEX` (and the `zIndex` prop entirely, 
nothing ever passed a custom value) instead of bumping it. Ant Design already 
auto-increments z-index for each newly opened `Modal` off 
`theme.zIndexPopupBase`. Since this modal is always opened on top of whatever 
it's interrupting, it stacks correctly with no manual number to maintain.
   
   Also audited every other `zIndex`/`z-index` usage in the frontend to see if 
this was a wider pattern. It isn't: this was the only place hardcoding a 
z-index override on a Modal-class component. The rest either don't need one, or 
already derive it from `theme.zIndexPopupBase`/`theme.zIndexBase` for real, 
local reasons unrelated to this bug (sticky headers inside modal content that 
need to stay above scrolled sibling content, the native Fullscreen API's 
interaction with portaled antd components, custom fixed-position overlays like 
the toast and chat widgets that sit outside Ant Design's own stacking system 
entirely). None of those needed to change.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A. I did try to spin up a local docker compose stack to grab real 
before/after screenshots, but hit an unrelated environment issue (a `Bus error` 
crashing both the webpack dev server and a jest run, most likely memory 
contention from another stack already running on the same machine) and couldn't 
get a live instance up. The original bug's before/after screenshots are already 
posted on #42546.
   
   ### TESTING INSTRUCTIONS
   Added a regression test (`UnsavedChangesModal.test.tsx`) that renders 
another open `Modal` alongside `UnsavedChangesModal` and asserts the latter's 
computed z-index is higher, i.e. it stacks on top without any hardcoded value. 
I wasn't able to run the frontend test suite locally in this environment (see 
above), so this hasn't been locally verified beyond code review, CI should 
confirm.
   
   Manually: open a dashboard or chart, open the View query/View SQL modal, 
then trigger an action that shows the unsaved-changes dialog (e.g. edit a 
control, then try to navigate away). The dialog should render on top.
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: #42510
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to