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.

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

refactor(app): confirm exit modal to appear over run details and run panel #9067

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Dec 10, 2021

closes #9053

Overview

This PR adds a portal to the confirm exit modal so it appears over the run details and run panel rather than only appearing over the run details.

Screen Shot 2021-12-10 at 11 42 04

Changelog

  • added top level portal

Review requests

  • make sure it matches AC of ticket

Risk assessment

low, behind ff

@jerader jerader requested a review from a team as a code owner December 10, 2021 16:45
@jerader jerader requested review from mcous and removed request for a team December 10, 2021 16:45
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #9067 (b6fa2f2) into edge (19df28b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #9067   +/-   ##
=======================================
  Coverage   74.76%   74.76%           
=======================================
  Files        1867     1867           
  Lines       49650    49650           
  Branches     4891     4891           
=======================================
  Hits        37120    37120           
  Misses      11673    11673           
  Partials      857      857           
Flag Coverage Δ
app 69.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
app/src/organisms/RunDetails/index.tsx 92.59% <ø> (ø)

@jerader jerader requested review from sakibh and b-cooper and removed request for mcous December 10, 2021 16:45
@jerader jerader added the app Affects the `app` project label Dec 10, 2021
@shlokamin
Copy link
Member

@emilywools should the background be black? I can't remember

@smb2268 smb2268 self-requested a review December 10, 2021 19:37
Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

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

Looks good! 🚀

@jerader jerader merged commit 2e66f1e into edge Dec 10, 2021
@jerader jerader deleted the app_run-details-confirm-exit-modal branch December 10, 2021 19:44
@emilywools
Copy link

full screen workflows (like robot calibration) use a black background. this modal background looks correct to me

mcous pushed a commit that referenced this pull request Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

confirm close modal should cover the whole app
4 participants