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): add InterventionModal to Error Recovery #15277

Merged
merged 8 commits into from
May 29, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented May 29, 2024

Closes EXEC-464

Overview

This PR adds the InterventionModal molecule to recovery flows.

The InterventionModal molecule required light refactoring to meet new designs and to have proper height/width on the ODD. There will be a pop-up modal-within-modal that the ER Intervention modal will need to support, but that is a separate ticket. Only the onClick functionality is added here.

Screen.Recording.2024-05-29.at.9.21.38.AM.mov

Test Plan

  • Verified InterventionModal matches designs linked above.

Risk assessment

low

@mjhuff mjhuff requested a review from a team as a code owner May 29, 2024 13:23
@mjhuff mjhuff requested review from TamarZanzouri and a team and removed request for a team May 29, 2024 13:23
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.

LGTM, nice!

app/src/molecules/InterventionModal/index.tsx Outdated Show resolved Hide resolved
@mjhuff mjhuff merged commit 58a43e7 into edge May 29, 2024
20 checks passed
@mjhuff mjhuff deleted the app_add-er-interventional-modal branch May 29, 2024 14:35
Comment on lines +21 to +22
import { getIsOnDevice } from '../../redux/config'

Copy link
Contributor

Choose a reason for hiding this comment

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

can you move this import before import type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh woops, I'll refactor this in my next PR 👍

{...HEADER_STYLE}
backgroundColor={headerColor}
justifyContent={headerJustifyContent}
onClick={props.iconHeadingOnClick}
Copy link
Contributor

@koji koji May 29, 2024

Choose a reason for hiding this comment

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

Can we destruct props instead of using props. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, can add in my next PR. Sorry I didn't see these in time 👍

mjhuff added a commit that referenced this pull request May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants