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

feat(app): run Disco animation after first-time-setup completion #13505

Merged
merged 3 commits into from
Sep 11, 2023

Conversation

fsinapi
Copy link
Contributor

@fsinapi fsinapi commented Sep 8, 2023

Overview

Very simple PR to add in a "disco" animation when the first time setup flow is completed, per status bar design specs. The confluence doc stated "user completes robot setup", so I just set it up to run when the user confirms the robot's name (see vid below).

Test Plan

Tested on Flexy McFlexFace

20230908_130235.mp4

Changelog

Review requests

Mostly want to make sure this is the correct place to put this.

Risk assessment

Pretty low

@fsinapi fsinapi requested a review from shlokamin September 8, 2023 19:10
@fsinapi fsinapi requested a review from a team as a code owner September 8, 2023 19:10
@fsinapi fsinapi self-assigned this Sep 8, 2023
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #13505 (de1ddd4) into chore_release-7.0.0 (017593d) will increase coverage by 0.00%.
Report is 2 commits behind head on chore_release-7.0.0.
The diff coverage is 83.33%.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##           chore_release-7.0.0   #13505   +/-   ##
====================================================
  Coverage                71.33%   71.33%           
====================================================
  Files                     2419     2418    -1     
  Lines                    67910    67914    +4     
  Branches                  7876     7876           
====================================================
+ Hits                     48443    48448    +5     
+ Misses                   17625    17624    -1     
  Partials                  1842     1842           
Flag Coverage Δ
app 68.88% <83.33%> (+0.01%) ⬆️

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

Files Changed Coverage Δ
...es/OnDeviceDisplay/RobotDashboard/WelcomeModal.tsx 91.66% <83.33%> (-8.34%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

because this component is the same rename component used in robot settings, the disco animation will also happen when you rename the robot from there.

i think what we probably want is for the WelcomeModal (which is what gets rendered right after you finish set up) to trigger the disco light on mount. you can do that by using the useEffect hook by passing it an empty dependency array. but also if you dont wanna deal with this someone from app ui can do it

@fsinapi
Copy link
Contributor Author

fsinapi commented Sep 8, 2023

I'm pretty sure that (at least on the 7.0.0 release branch) this component is only used at the end of the setup flow - the settings page just has the one page with a textbox and then goes back to the settings page once a name is entered.

Either way, I agree that syncing this to show up with the welcome modal makes more sense, will do 👍

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

🥳

@fsinapi fsinapi merged commit af8aae5 into chore_release-7.0.0 Sep 11, 2023
@fsinapi fsinapi deleted the RCORE-781_disco_after_first_time_setup branch September 11, 2023 13:11
jerader pushed a commit that referenced this pull request Sep 12, 2023
)

* App sends "setStatusBar" command to run "disco" animation after first-time setup completes
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.

2 participants