Skip to content

fix(app): Show module setup screen on the Desktop app for the given module. #18487

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

Merged
merged 22 commits into from
Jun 4, 2025

Conversation

vegano1
Copy link
Contributor

@vegano1 vegano1 commented Jun 2, 2025

Overview

Show the "Setup Module" screen when setting up a module on the Desktop app.
Closes: RQA-4208

Screenshot 2025-06-02 at 2 42 12 PM Screenshot 2025-06-02 at 2 50 27 PM

Test Plan and Hands on Testing

  • Make sure that clicking on the "Setup Module" notification on the desktop app shows the "Setup Module" screen for the given module, even if there are multiple modules that need to be set up.
  • Make sure the "Success" screen at the end of the flow does NOT show the "Setup another module" button for the desktop app.
  • Make sure the "Setup module" toast on the ODD is not shown if a run is in progress
  • Make sure the changes dont affect protocol setup, etc

Changelog

  • Show the "Setup Module" screen on the desktop app for the given module.
  • Don't show the "Setup another module" button in module setup for the desktop app.
  • Don't show the "Setup module" toast on the ODD if a run is in progress.

Review requests

  • Does this make sense?

Risk assessment

Low, unreleased

vegano1 added 14 commits May 23, 2025 15:53
- add setup another module button + logic to success screen
- fix some screens that werent using the correct SmallButton vs PrimaryButton
- blink module when we load the module setup launcher screen
- add useSendIdentifyModule to new utils.ts file so we can reuse createLiveCommands
- blink module red if install detection fails during module setup launcher
- blink module blue when the device re-connects after a firmware update
- stop blinking module when module setup is complete and the modal exits
- block moduleSetup exit when module is updating
- stop blinking module if the flow is closed while in the SelectModule component
- show the New Module Setup screen when launching module setup from the ModuleCard component
- correctly start/stop blinking when retrying after InstallDetect failure
- Make sure we are correctly changing the LED state when setting up multiple modules
@vegano1 vegano1 requested a review from a team as a code owner June 2, 2025 18:51
@vegano1 vegano1 requested review from TamarZanzouri and removed request for a team June 2, 2025 18:51
@vegano1 vegano1 changed the title Rqa 4208 show module attached screen fix(app): Show module setup screen on the Desktop app for the given module. Jun 2, 2025
Copy link

codecov bot commented Jun 2, 2025

Codecov Report

Attention: Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.

Project coverage is 25.24%. Comparing base (0e1ba8f) to head (b8ee6a0).
Report is 5 commits behind head on edge.

Files with missing lines Patch % Lines
...p/src/organisms/ModuleWizardFlows/SelectModule.tsx 0.00% 8 Missing ⚠️
app/src/organisms/ModuleWizardFlows/index.tsx 0.00% 8 Missing ⚠️
app/src/App/hooks.ts 0.00% 5 Missing ⚠️
app/src/organisms/ModuleWizardFlows/Success.tsx 0.00% 2 Missing ⚠️
app/src/App/OnDeviceDisplayApp.tsx 0.00% 1 Missing ⚠️
app/src/organisms/ModuleCard/index.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #18487       +/-   ##
===========================================
- Coverage   57.47%   25.24%   -32.23%     
===========================================
  Files        3269     3270        +1     
  Lines      280761   279640     -1121     
  Branches    33021    32981       -40     
===========================================
- Hits       161355    70605    -90750     
- Misses     119210   209007    +89797     
+ Partials      196       28      -168     
Flag Coverage Δ
app 1.25% <0.00%> (-46.17%) ⬇️

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

Files with missing lines Coverage Δ
app/src/App/OnDeviceDisplayApp.tsx 0.00% <0.00%> (-76.55%) ⬇️
app/src/organisms/ModuleCard/index.tsx 0.00% <0.00%> (-60.67%) ⬇️
app/src/organisms/ModuleWizardFlows/Success.tsx 0.00% <0.00%> (-12.83%) ⬇️
app/src/App/hooks.ts 0.00% <0.00%> (-15.70%) ⬇️
...p/src/organisms/ModuleWizardFlows/SelectModule.tsx 0.00% <0.00%> (-1.42%) ⬇️
app/src/organisms/ModuleWizardFlows/index.tsx 0.00% <0.00%> (-8.99%) ⬇️

... and 1759 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@ahiuchingau ahiuchingau left a comment

Choose a reason for hiding this comment

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

Nice thank you!

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

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

Nice!

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 don't think we should add a current run poll to the top level like that - we're really trying to cut down on those. What if we instead check to see if there's a current run only when we recieve a module-connected event?

@vegano1
Copy link
Contributor Author

vegano1 commented Jun 3, 2025

I don't think we should add a current run poll to the top level like that - we're really trying to cut down on those. What if we instead check to see if there's a current run only when we recieve a module-connected event?

Agreed, however, the useCurrentRunId hook uses the useNotifyAllRunsQuery which should only refetch if we get a notification.

@vegano1 vegano1 requested a review from sfoster1 June 3, 2025 17:19
vegano1 added 5 commits June 3, 2025 17:13
- dont return optional Null from JSX component, let the caller deal with conditonal rendering
- dont define react components constants mid code, then use them in the return (render) function
- replace console.error usage with setErrorMessage as these errors are non-recoverable for this flow + lets us get rid of returning null
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.

All right, sounds good to me!

Base automatically changed from EXEC-1549-setup-another-module to edge June 4, 2025 13:51
@vegano1 vegano1 merged commit 8c92593 into edge Jun 4, 2025
30 of 32 checks passed
@vegano1 vegano1 deleted the RQA-4208-show-module-attached-screen branch June 4, 2025 16:01
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.

4 participants