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/CropProbability-Guided Tour #190

Closed
wants to merge 18 commits into from
Closed

Feat/CropProbability-Guided Tour #190

wants to merge 18 commits into from

Conversation

FaithDaka
Copy link
Collaborator

Description

  • Implementation of crop probability guided tour to include a tour with user interactions.
  • A user will be able to start a tour and be guided along as they interact with the tool

Preview

  • N/A

Tour Video

CP-Guided.Tour.-.Made.with.Clipchamp.mp4

Copy link
Collaborator

@chrismclarke chrismclarke left a comment

Choose a reason for hiding this comment

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

Thanks @FaithDaka
I spent quite a bit of time trying to test out the tour functionality and can see how there's actually a lot of challenges with intro.js

A few things in particular which you probably already noticed:

  1. Intro.js evaulates all step elements at start, so breaks if using elements that are added to the dome later. I'm guessing that is why you had to create multiple separate tours to trigger at different times

  2. Elements that use popups/modals are blocked from interaction (e.g. mat-select).

  3. Tours don't work when triggered from inside some elements (e.g mat-tab element used when embedding tools on farmer activity pages)

  4. Focusing large elements seems to get scroll positioning wrong (e.g. table element which can be larger than screen, guessing that's why you focused specific cells within the table)

  5. Hard to track tour state as user interacts with elements

I'm going to work to see if I can find tidier solutions for these issues in #194, building on the work already done in this PR and so will just block this from merge until able to sort the rest

@chrismclarke chrismclarke mentioned this pull request Nov 1, 2023
@FaithDaka
Copy link
Collaborator Author

Yes @chrismclarke
The library seems to be built for straight-forward usage and doesn't marry well in complex use-cases. I found a few work arounds but will be happy to see your approach as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants