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

Fix tour #1049

Merged
merged 14 commits into from Apr 29, 2022
Merged

Fix tour #1049

merged 14 commits into from Apr 29, 2022

Conversation

rwblair
Copy link
Collaborator

@rwblair rwblair commented Apr 25, 2022

Not sure why withRouter broke, but I updated Tour to be a proper component wrapped in withRouter and added typing information as necessary.

@rwblair
Copy link
Collaborator Author

rwblair commented Apr 25, 2022

Because of how steps is conditionally defined by pathname there is a bug if you go through the tour into the builder section, then close the tour, then restart it via the drop down, current step will be greater than 3 but the steps array is only 4 long when pathname is builder. Need to reset step count on reopen tour, or not redirect to home on starting it.

@cypress
Copy link

cypress bot commented Apr 25, 2022



Test summary

8 0 0 0Flakiness 0


Run details

Project Neuroscout
Status Passed
Commit c573964 ℹ️
Started Apr 29, 2022 6:20 PM
Ended Apr 29, 2022 6:21 PM
Duration 01:49 💡
OS Linux Ubuntu - 20.04
Browser Electron 94

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@adelavega
Copy link
Collaborator

Awesome, can you deploy to DD?

@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2022

Codecov Report

Merging #1049 (f55c85b) into master (b6d668b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1049   +/-   ##
=======================================
  Coverage   78.23%   78.23%           
=======================================
  Files          63       63           
  Lines        3272     3272           
=======================================
  Hits         2560     2560           
  Misses        712      712           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6d668b...f55c85b. Read the comment docs.

@rwblair
Copy link
Collaborator Author

rwblair commented Apr 28, 2022

@adelavega should be up on deepdream

@adelavega
Copy link
Collaborator

adelavega commented Apr 28, 2022

Looks really good.

Two issues:

  1. Log in is acting funny (at least on deepdream). It doesn't close the log in window.
    image

  2. Minor: If you start an analysis and it telsl you "you sure you want to leave?" and you say "cancel" it'll start the tour right there.

That said, the behavior is sane, because it highlights the top level elements, and then can't start the Builder.
Not horribly bad, but it is odd, so I guess ideally it wouldn't start the tour if you click "Cancel"

@adelavega
Copy link
Collaborator

Reminder: only changes needed are fixing tests, and changing the color of the flashy tour button.

@rwblair rwblair merged commit 5203cab into master Apr 29, 2022
@adelavega adelavega mentioned this pull request Jun 1, 2022
@adelavega adelavega deleted the fix/tour branch September 22, 2022 21:58
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.

None yet

3 participants