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

Missing 'All classes' link on device with multiple facilities #10673

Closed
pcenov opened this issue May 10, 2023 · 6 comments · Fixed by #10768
Closed

Missing 'All classes' link on device with multiple facilities #10673

pcenov opened this issue May 10, 2023 · 6 comments · Fixed by #10768
Assignees
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend P1 - important Priority: High impact on UX

Comments

@pcenov
Copy link
Member

pcenov commented May 10, 2023

Observed behavior

The 'All classes' link is missing on a device with multiple facilities so once I have selected one of the available classes I have to navigate to a different plugin in order to go back to the Classes page.

Expected behavior

The 'All classes' link should be displayed in that case.

2023-05-10_16-52-00

general expectations for Coach behavior & UI:

  • if user 1 is a multi-facility admin of Facility A & Facility B, and Facility A has only one class, Class home goes directly to that class's page and when there, the back link is "← Change learning facility"
  • when user 1 selects Facility B, which has multiple classes, they are redirected to the All Classes page which has a back link to "← Change learning facility".
    • selecting Class B1 will take them to its specific class page, which contains a back link to "← All classes"
  • if user 2 has access to only Facility A, which has one class (or user has other limitations only allowing access to one class), Class home goes directly to that class's page and there is no back link displayed.
  • if user 3 has access to only Facility B, which has multiple classes, they are redirected to the All Classes page and there is no back link displayed.
    • selecting Class B1 will take you to its specific class page with a back link to "← All classes"

Steps to reproduce the issue

  1. Install the the build asset from this PR
  2. Import a facility with multiple classes
  3. Navigate to Coach > Class home and select a class
  4. Attempt to go back to the Classes page

Note: observed in edge case when multi-facility user's main facility has 1 class and imported facility has multiple, detailed in comments

Video

2023-05-10_16-30-45.mp4

Usage Details

Ubuntu 22 - Chrome, Firefox

@pcenov
Copy link
Member Author

pcenov commented May 10, 2023

@radinamatic

@radinamatic radinamatic added P1 - important Priority: High impact on UX APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels May 11, 2023
@thanksameeelian thanksameeelian self-assigned this May 15, 2023
@thanksameeelian
Copy link
Contributor

thanksameeelian commented May 15, 2023

i'm actually unable to recreate this problem because another bug is keeping me from being able to see this view at all: #10643. when i initially mentioned the linked bug, which i encounter every time i try to navigate within Coach > Class Home without fail, @marcellamaki mentioned she believed it was related to routing work that was still ongoing within Coach.

@pcenov, have you ever encountered the bug detailed in #10643 (& in the screen recording below)? because i consistently encounter it, i'd consider this issue "blocked" but now i'm curious if i'm the only one experiencing it! 🙂 (please see update below!)

cant-navigate-within-coach-classes.mov

@thanksameeelian
Copy link
Contributor

following up on my comment after a conversation with @rtibbles sorted things out somewhat: it appears that i'm experiencing the bug in #10643 & the screen recording because all of the classes i'm attempting to access contain quizzes.

in the examples used to illustrate this "missing 'all classes' link" issue, it appears that the classes don't contain quizzes, so navigation to them isn't triggering the same bug, which seems to be occurring within a code block that is never accessed if no quizzes exist.

no action or anything needed, i just wanted to clarify my earlier comment! i'll be working on this but likely only once #10643 is addressed, which should be quite soon.

@thanksameeelian
Copy link
Contributor

@pcenov i haven't been able to exactly replicate the issue reported here, as throughout my navigation within classes in Coach as a multi-facility user i've been consistently seeing the "← All classes" back links as expected if there are multiple classes. but there is some behavior proposed in #10650 that i think aligns with the sentiment of this issue and could be addressed in response to this issue instead:

  • change backlink text from "← All Facilities" to "← Change learning facility" (in commonCoreStrings)
  • add backlink to all main Coach subtopic pages, not just "Class home"

(changing the text to align with the recent update to the same type of link in the Facility plugin)

in talking through this issue with @rtibbles, the following rules were laid out to help govern navigation behavior in Coach and establish expectations:

  • if user 1 is a multi-facility admin of Facility A & Facility B, and Facility A has only one class, Class home goes directly to that class's page and when there, the back link is "← Change learning facility"
  • when user 1 selects Facility B, which has multiple classes, they are redirected to the All Classes page which has a back link to "← Change learning facility".
    • selecting Class B1 will take them to its specific class page, which contains a back link to "← All classes"
  • if user 2 has access to only Facility A, which has one class (or user has other limitations only allowing access to one class), Class home goes directly to that class's page and there is no back link displayed.
  • if user 3 has access to only Facility B, which has multiple classes, they are redirected to the All Classes page and there is no back link displayed.
    • selecting Class B1 will take you to its specific class page with a back link to "← All classes"

i was planning on adding these expectations to the description of this issue (and removing the related bullet points from #10650), because it seemed like the specific issue you raised, even if i wasn't able to recreate it exactly, should be appropriately handled by a PR that enforces these expectations throughout Coach.

i wanted to check in with you before editing though, in case you felt like the case you're experiencing isn't adequately covered by this, you feel my interpretation is incorrect, or you have any other objections to potentially expanding the scope of this issue slightly.

@pcenov
Copy link
Member Author

pcenov commented May 19, 2023

Hi @thanksameeelian, thank you for your detailed investigation on this one and yes, I believe the use cases you've outlined cover nicely the expected behavior for Coach.

I was also able to identify that the issue I've reported is only valid if there's a single class in my Main facility and there are multiple classes in the imported facility. I also noticed that if I use the browser's Back and Forward buttons then the All classes link gets displayed. Here's a video of that:

2023-05-19_12-34-39.mp4

@thanksameeelian
Copy link
Contributor

thanksameeelian commented May 19, 2023

great! i think i have my work cut out for me then :) and thank you @pcenov for defining the bug with such precision - the combinations of things i'd tried just weren't quite producing it and this is really helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend P1 - important Priority: High impact on UX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants