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

[FAB-133] HiFi Canvas Tutorial #55

Merged
merged 25 commits into from
Oct 5, 2021
Merged

Conversation

nicolauscg
Copy link
Member

@nicolauscg nicolauscg commented Oct 2, 2021

features

  • tutorial for canvas drawing screen, steps includes:
    • mention of tutorial button in navbar to access it again
    • drawing follows theme and student is assigned a specific role (chip row above canvas)
    • can only see own drawing, to view combined drawing, see hub screen
    • explain each tool available to story and drawing roles
  • remember when tutorial is accessed and do not automatically show prompt again for 1 day
  • hide login, register, profile, logout button for student pages i.e. when url is /join
  • fableous logo in navbar not clickable when student on drawing session

bugs found

  • on short screens, when canvas toolbar y-axis overflows to scroll, its scroll position does not align when tutorial is explaining the brush tool
    • tried all props related to scroll and does not seem to change any behavior, so since this is a minor bug only present on small screens, can consider leaving it
  • manually starting tutorial through navbar button shows first step of tutorial, which is asking "do you want to go through the tutorial?", its kinda akward
  • tutorial step with center placement causes scrollbar to appear on canvas drawing screen

out of scope

  • matching UI of tooltip to prototype, could be taken by Will for his gallery tutorial task
  • review and finalising tutorial steps, wording

@daystram daystram temporarily deployed to Development October 2, 2021 12:00 Inactive
@daystram daystram temporarily deployed to Development October 2, 2021 12:00 Inactive
@daystram daystram temporarily deployed to Development October 2, 2021 12:02 Inactive
@daystram
Copy link
Contributor

daystram commented Oct 2, 2021

We could also hide the profile and logout button, and disable the Fableous logo button when the user is in the canvas page.

@daystram daystram temporarily deployed to Development October 3, 2021 05:06 Inactive
@daystram daystram temporarily deployed to Development October 3, 2021 05:06 Inactive
@daystram daystram temporarily deployed to Development October 3, 2021 05:08 Inactive
@nicolauscg
Copy link
Member Author

We could also hide the profile and logout button, and disable the Fableous logo button when the user is in the canvas page.

good idea, at the moment I only hid login register buttons, will do the rest in a moment

@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

This is also an apparent bug, viewport scrolls when tooltip appears
ss

@nicolauscg
Copy link
Member Author

previous two comments by Dan have been addressed

@nicolauscg nicolauscg marked this pull request as ready for review October 3, 2021 10:07
@daystram daystram changed the title [FAB-133] Feat/hifi tutorial canvas [FAB-133] HiFi Canvas Tutorial Oct 3, 2021
@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

Rename localStorage.ts to storage.ts?

@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

For the IDs in tutorialTargetIds.ts, how about making them all enums?

@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

Other than that, all G

@daystram daystram temporarily deployed to Development October 3, 2021 15:21 Inactive
@daystram daystram temporarily deployed to Development October 3, 2021 15:21 Inactive
@daystram daystram temporarily deployed to Development October 3, 2021 15:24 Inactive
@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

I can't figure out why the spotlight on the eraser button has no padding while the other has.
image
image

@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

I wonder if the transition animation could be made less jerky than this

Screen.Recording.2021-10-04.at.1.32.45.am.mov

@daystram
Copy link
Contributor

daystram commented Oct 3, 2021

The tutorial should also be cancelled when the page state changes. Currently, if the drawing finishes or resets back to the form page, the tutorial overlay remains

@daystram daystram temporarily deployed to Development October 4, 2021 07:01 Inactive
@daystram daystram temporarily deployed to Development October 4, 2021 07:01 Inactive
@daystram daystram temporarily deployed to Development October 4, 2021 07:04 Inactive
@nicolauscg
Copy link
Member Author

  • renamed localStorage.ts to storage.ts, function names are still kept the same
  • combined ids to one enum in tutorialTargetIds.ts
  • fixed eraser icon to have same size with the rest, the inconsistency before was because there is no eraser icon in material ui, so I used one found on the internet
  • disabled animation for tutorial tooltip, yes it does look weird
  • tutorial will correctly close when controllerState changes

@sonarcloud
Copy link

sonarcloud bot commented Oct 5, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@daystram daystram temporarily deployed to Development October 5, 2021 07:06 Inactive
@daystram daystram temporarily deployed to Development October 5, 2021 07:06 Inactive
@daystram daystram temporarily deployed to Development October 5, 2021 07:10 Inactive
@daystram daystram merged commit 92dc429 into master Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants