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

Feature quiz section tabs with overflow #11382

Conversation

nucleogenesis
Copy link
Member

@nucleogenesis nucleogenesis commented Oct 10, 2023

Summary

  • Introduces a component to accommodate design requiring that tabs stay on one line such that overflowing tabs are presented in a drop-down menu. This involved what feels a little bit like <slots> Inception so hopefully it isn't too squirrely
  • The natural way that I found to accommodate keyboard users accessibly jives with the inspiration I took from the HorizontalMenuWithOverflow component where we use overflow: hidden and flex-wrap to allow the tabs to render out of sight. We calculate "which tabs are out of sight?" then present those items in the dropdown menu. The accessible solution here seemed to be to put tabindex=-1 onto the overflow dropdown such that it will not be focused. Instead, the focus just continues onto the hidden tabs, showing them visibly.
  • Changes to the side panel so that it is in a better context for managing focus on close
  • Uses the new messageLabel$ convention for strings.

NOTE: This uses this KDS PR

References

Closes #11274

Reviewer guidance

Apologies for lack of videos - I am having issues recording things right now.

  • Please test screen reader and keyboard navigation from top to bottom.
  • Test opening/closing and navigating each section tab's context menu and each of the actions available there
  • Add enough sections to the quiz such that the overflow dropdown appears.
  • The dropdown and its option should not be focusable by tabbing. As you navigate through the section tabs while the overflow is present, you should still be able to keyboard navigate to all of the tabs.
  • You should be able to access context menus for sections in the overflow dropdown
  • @tomiwaoLE note that to indicate that a focused item is in the overflow list, a primary colored circle surrounds the dropdown icon button:
    image

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: medium labels Oct 10, 2023
@nucleogenesis nucleogenesis marked this pull request as ready for review October 20, 2023 05:14
Copy link
Member

@LianaHarris360 LianaHarris360 left a comment

Choose a reason for hiding this comment

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

This is a very exciting feature, it already looks amazing! A couple of things I noticed was that the KLinearLoader remains on the page even after the quiz creation section has finished loading. I saw that the CoachImmersivePage accepts a loading prop that might need to be provided.

Also, when you exit out of creating a quiz without saving, and then select "new quiz" again, although the array of tabs appears to be the correct length, the section counter continues as if you never exited out of creating the quiz. It does correctly reset on page reload.

@nucleogenesis nucleogenesis merged commit 6dc82f3 into learningequality:develop Oct 25, 2023
35 checks passed
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.) SIZE: medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants