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

Guided tour #112

Merged
merged 5 commits into from
Mar 21, 2021
Merged

Guided tour #112

merged 5 commits into from
Mar 21, 2021

Conversation

JadeMaveric
Copy link
Contributor

@JadeMaveric JadeMaveric commented Mar 14, 2021

This addresses the issue laid out in #111
React-Joyride is used to generate the tour.

There's 5 stages in the tour: Title, Description, AvailableTimes controls, AvailableTimes calendar and the submit button.

A simple localStorage variable is used to detect if the user has already visited the page. (The poll page is double loading - due to some preexisting code. It's noticeable since the popup reloads along with the page)

There's also a help button, so that users can trigger the tour manually. And I matched the color scheme to the res of the site. (This one line may need to be refactored as part of #56)

I couldn't find any design docs, but I've tried to use the same coding style that was already present. Typescript typings et all (thankfully, joyride provides those)

By default the tour scrolls to the relevant elements, this causes a minor disturbance. The scrolling can be disabled, but a better fix is to adjust the height of the page to the viewport.

The code doesn't affect any existing modules, but I still tested it against a production build. Everything looks fine.
rocket-meet-guided-tour

@JadeMaveric JadeMaveric marked this pull request as ready for review March 14, 2021 10:26
@anandbaburajan
Copy link
Member

Thank you @JadeMaveric! :D @VipinVIP can you check out the tour and let us know what you think? Thanks!

@anandbaburajan anandbaburajan linked an issue Mar 15, 2021 that may be closed by this pull request
Copy link
Contributor

@VipinVIP VipinVIP left a comment

Choose a reason for hiding this comment

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

Very good work. Everything looks neat, and thankyou for that extra effort to find and use the matching colour scheme. 😃

Will merge right after i get an OK from @anandbaburajan

@VipinVIP
Copy link
Contributor

The scrolling can be disabled, but a better fix is to adjust the height of the page to the viewport.

Yep. I guess hat viewport issue can also be considered in #62

@JadeMaveric
Copy link
Contributor Author

Thanks! You guys are really supportive, pleasure working with you :D

@anandbaburajan
Copy link
Member

I checked it out, it's really nice, thanks! Some comments: please lint your code and try to fix those warnings/errors. Most of them are trivial and I think Prettier would help. You can also setup auto linting in vscode. Btw, we'll soon have a ESLint checker, thanks to @aysp-99. Also, @anaswaratrajan can you take a look and check if it would be good to use Redux here? Thanks!

@JadeMaveric
Copy link
Contributor Author

Alright, I'll go through them and push a commit. I could have sworn I setup auto linting 🙆🏼‍♂️

Copy link
Contributor

@VipinVIP VipinVIP left a comment

Choose a reason for hiding this comment

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

All the linting errors have been solved and its good to go. 😇

@anandbaburajan
Copy link
Member

@JadeMaveric cool! @anaswaratrajan will soon let us know if using Redux here would be a good idea.

@anandbaburajan
Copy link
Member

All the linting errors have been solved and its good to go.

@JadeMaveric can rebase and force-push to see the ESLint checker in action here, if he wishes.

@JadeMaveric
Copy link
Contributor Author

Am I missing something? I don't see any gh action in action

@anandbaburajan
Copy link
Member

Am I missing something? I don't see any gh action in action

Haha, my bad. #109 is still WIP.

@anandbaburajan
Copy link
Member

Hey @JadeMaveric, we would be happy if you could check our texts at Gitter. We've an upcoming meet!

@anastr0
Copy link
Contributor

anastr0 commented Mar 21, 2021

@anaswaratrajan can you take a look and check if it would be good to use Redux here?

Using the store might be an overkill since the flag is required in only 1 file. No need of global state.

@JadeMaveric Thanks! We really needed this. Looks good. :D
And sorry for the late comment.

@anandbaburajan
Copy link
Member

Cool, merging! Thanks!

@anandbaburajan anandbaburajan merged commit 25a4f4a into samay-app:main Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guided tour for first time users
4 participants