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

fix(app): place top portal at higher z index than page level portal #6950

Merged
merged 2 commits into from
Nov 9, 2020
Merged

fix(app): place top portal at higher z index than page level portal #6950

merged 2 commits into from
Nov 9, 2020

Conversation

b-cooper
Copy link
Contributor

@b-cooper b-cooper commented Nov 9, 2020

Overview

In order to ensure that top level modals render on top of any page level modals, this sets the z
index of the top level portal root's contents to 10 as opposed to 1 for the page level portal

Changelog

  • add zIndex of 10 to top level portals

Review requests

  • pretty hard to test, but you can start any one of the new calibration flows and then turn off wifi or unplug from usb, you should see the disconnect modal appear behind the session modal for an instant before the whole page unmounts. The edge case that this is really preventing is the app update modal appearing above the active calibration modal.

Risk assessment

low

@b-cooper
fix(app): place top portal at higher z index than page level portal
38c2fe1 
In order to ensure that top level modals render on top of any page level modals, this sets the z
index of the top level portal root's contents to 10 as opposed to 1 for the page level portal
@b-cooper b-cooper requested a review from a team as a code owner November 9, 2020 21:51
@b-cooper b-cooper requested review from mcous and removed request for a team November 9, 2020 21:51
@b-cooper b-cooper added this to the HMG Sprint 22 milestone Nov 9, 2020
@b-cooper b-cooper added app Affects the `app` project hmg hardware, motion, and geometry ready for review labels Nov 9, 2020
@b-cooper b-cooper requested review from a team and ahiuchingau and removed request for a team November 9, 2020 21:52
@nusrat813 nusrat813 self-requested a review November 9, 2020 22:14
sfoster1
sfoster1 approved these changes Nov 9, 2020
View reviewed changes
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

I love a good z index

nusrat813
nusrat813 approved these changes Nov 9, 2020
View reviewed changes
Copy link

@nusrat813 nusrat813 left a comment

Choose a reason for hiding this comment

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

PR fixes the bug
note: following PR will address session closing when a disconnect occurs

@b-cooper
Copy link
Contributor Author

b-cooper commented Nov 9, 2020

NOTE: When testing this PR @nusrat813 noticed another behavioral bug that leads to a potential dead end if the app disconnects from the robot during a calibration flow. I am beginning work now on a separate PR that handles the deletion of all sessions in redux state as soon as a robot becomes unreachable.

@b-cooper b-cooper merged commit b08c4d3 into chore_release-4.0.0-beta.0 Nov 9, 2020
@mcous mcous deleted the app_portal-tweaks branch June 10, 2022 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nusrat813 nusrat813 nusrat813 approved these changes

@sfoster1 sfoster1 sfoster1 approved these changes

@mcous mcous Awaiting requested review from mcous

@ahiuchingau ahiuchingau Awaiting requested review from ahiuchingau

Assignees
No one assigned
Labels
app Affects the `app` project hmg hardware, motion, and geometry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@b-cooper @sfoster1 @nusrat813