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

Add guided tour / walkthrough #2168

Merged
merged 78 commits into from Jul 28, 2022
Merged

Add guided tour / walkthrough #2168

merged 78 commits into from Jul 28, 2022

Conversation

hughrun
Copy link
Contributor

@hughrun hughrun commented Jul 3, 2022

This is intended to address #2052

I expect to have to make changes in response to feedback for this, so please provide any you have. I've tried to cover all the basics without overwhelming new users. I may not have gotten that balance right.

Uses Shepherd (MIT license). My understanding is that this is pretty good with accessibility, hopefully that's correct.

I have set this up so that it should pick up styles from the active theme, whatever it is. This requires some slightly hacky work on the default Shepherd CSS, so I'm happy to receive any feedback on that if I messed it up.

All elements used to anchor guided tour modals have an id - if I added the id (which is the case for nearly all of them) I've used a tour- prefix to make it harder to accidentally mess up the tour and not fix when inevitably things get moved around in future. All tour wording should be translatable, but it would be great if someone can double check that.

Every user will now have a show_guided_tour setting, set to True by default. This is easy to turn off on the first pop up modal on every page the tour appears on. Users can trigger the tour (i.e. turn their setting back to True) by clicking on a link in the footer. If JavaScript is disabled a message will appear next to this link noting that JS is required for the tour.

Adds a sass file based on the v10.0.0 Shepherd CSS. Original Shepherd styles are kept where appropriate, otherwise this is intended to inherit whatever styles are being used through the Bulma and Bookwyrm SASS, so that it uses appropriate colours in both light and dark modes.
- include logic in main layout to add button if there is a page tour available
- add button for main user feed page
This file creates and triggers tours using shepherd.

Initially this is a tour on the home feed page, triggered by clicking on the help button in the top nav.
This boolean value indicates whether the user wishes to be show the guided tour.
It defaults to True but will be able to be easily set to False.
Uses a URL param to indicate whether the value should be set to True or False.
Redirects to home page.
Use a url fragment (<tour>) instead of a classic url param (/?tour=True)
This uses an embedded script tag so that we can use django templates for logic - most importantly, we need to be able to use translations within the tour text.
- adds ids to relevant elements to enable tour
- adds guided tour using Shepherd
The first pop up in the guided tour on each page should provide a button to switch off the guided tour altogether, not simply cancel the current iteration.
If we don't do this, then the only way to turn off the guided tour is to go right to the end, which could be really irritating, especially for people who star the tour and then start exploring on their own.
- add ID for add group button
- add tour steps for user groups page
- trigger tour steps if guided tour is turned on
Includes adding creating a new group.
Move cancel button function into a separate JS file.
The selector JS for this function cannot be within bookwyrm.js because the guided tour elements load after bookwyrm.js.
Adds a guided tour for book search page including logic for differing messages depending on what results are visible.
This is intended to be one of the earlier pages in the tour. It should show users the concept of reading status, editions, and other useful points.
Takes user through the main /list page, as well as the options for creating a list.
@hughrun
Copy link
Contributor Author

hughrun commented Jul 10, 2022

There are still some tests failing. Seems I may have broken something to do with views, not sure what.

@hughrun hughrun requested a review from mouse-reeve July 10, 2022 01:42
@hughrun hughrun marked this pull request as ready for review July 10, 2022 01:42
@mouse-reeve
Copy link
Member

It looks like the tests are all html validation errors caused by elements with id tour-spoiler-alert appearing multiple times on the same page

Copy link
Member

@mouse-reeve mouse-reeve left a comment

Choose a reason for hiding this comment

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

This looks great and works great, just some quibbling about wording and steps :)

bookwyrm/templates/guided_tour/home.html Outdated Show resolved Hide resolved
bookwyrm/templates/guided_tour/user_profile.html Outdated Show resolved Hide resolved
bookwyrm/templates/guided_tour/direct_messages.html Outdated Show resolved Hide resolved
bookwyrm/templates/guided_tour/direct_messages.html Outdated Show resolved Hide resolved
@hughrun
Copy link
Contributor Author

hughrun commented Jul 15, 2022

thanks for the review, I'll make some adjustments in the next day or two.

- new classname for posting guide
- various improvements to wording
- use function to find responsive menu elements
- add scrollTo transitions where needed
- we need to do this because of conflicting migrations
Show_guided_tour needs to come after alter_user_preferred_language due to conficts. I think.
@hughrun
Copy link
Contributor Author

hughrun commented Jul 17, 2022

Ok I think I have addressed all concerns now. I also fixed an issue with responsive menu elements not being picked up in the tours when they became hidden.

Assuming @mouse-reeve is happy with this, there's a couple of others things to do to cover this off:

  1. Instance admins will need to run ./bw-dev collectstatic in addition to the migrations to make the tour work, because it amends the JS and CSS files.
  2. We need to decide and document somewhere what the process will be when interface or functional changes are made that require the guided tour to be amended, and also when a new feature is added and how we decide whether/where/how to add that to the tour.

@hughrun hughrun requested a review from mouse-reeve July 17, 2022 06:52
@mouse-reeve
Copy link
Member

collectstatic is run as part of the instance update command, so that shouldn't be an issue (if people aren't using that command, they'll have run into problems long before now).

It should be possible to get unit tests to verify that the tour elements on the page match up to elements on the rendered page, which would be a basic way to at least see if there's been a code change that disconnects part of the tour. I think this could be added to the helper function that checks html validity in the unit could do this. #2201 will be a good test case of changing something in the UI that's part of the tour.

Copy link
Member

@mouse-reeve mouse-reeve left a comment

Choose a reason for hiding this comment

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

I hate requesting changes again because this is rad and I'd like to get it merged, I just can't figure out why that one step isn't showing up. Am I missing something?

But pending that I think it's great and I'm very appreciative of your patience while I haven't been the most responsive

bookwyrm/templates/guided_tour/home.html Outdated Show resolved Hide resolved
@hughrun
Copy link
Contributor Author

hughrun commented Jul 27, 2022

I hate requesting changes again because this is rad and I'd like to get it merged, I just can't figure out why that one step isn't showing up. Am I missing something?

But pending that I think it's great and I'm very appreciative of your patience while I haven't been the most responsive

No problem - I realised about halfway through coding this all up that I had created a never-ending backlog of updates, given the app will change haha

Lemme take a look at this, I suspect it may be to do with the responsive design.

Fixes bug on larger screens.

We need to use a function to set the anchor for tour steps when using menus and other elements that become visible or hidden responsively. Because the element is still in the DOM, we can't just rely on it disappearing completely, we have to assign a different (visible) element otherwise the step will simply disappear and the user cannot continue the tour. Previously this used a simple selector which didn't work due to the above.
A couple of tour steps could benefit from a scrollTo for users on smallers screens.
@hughrun hughrun requested a review from mouse-reeve July 27, 2022 05:56
@hughrun
Copy link
Contributor Author

hughrun commented Jul 27, 2022

It should be possible to get unit tests to verify that the tour elements on the page match up to elements on the rendered page, which would be a basic way to at least see if there's been a code change that disconnects part of the tour. I think this could be added to the helper function that checks html validity in the unit could do this. #2201 will be a good test case of changing something in the UI that's part of the tour.

Let me know if you'd like me to work on this also - I may need some initial pointers re how it works at the moment.

@mouse-reeve
Copy link
Member

I think that should be a follow-up PR, and I'm happy to do it, it will be fussy and require knowing specifics about how I set up the tests that aren't that interesting

Copy link
Member

@mouse-reeve mouse-reeve left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you :)

@mouse-reeve mouse-reeve merged commit ed20587 into bookwyrm-social:main Jul 28, 2022
@hughrun hughrun deleted the tour branch January 14, 2024 01:11
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

2 participants