Skip to content

Make Manual Question Selection alert non-sticky on scroll #13331

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

Merged

Conversation

LianaHarris360
Copy link
Member

Summary

This pull request divides the class alert-warning into two classes to ensure that the manual question selection banner is not sticky, while the banner for the selected maximum content remains sticky when scrolling in the SidePanelModal.

Before:

Before.mov

After:

After.mov

References

Fixes #13299

Reviewer guidance

  1. As a coach, create a new quiz
  2. Add questions to a section
  3. Select or unselect the option to manually select questions to see the manualSelectionOnNotice or manualSelectionOffNotice
  4. Open content with enough questions to scroll down the side panel modal. The manual selection notice should not be sticky when scrolling

@LianaHarris360 LianaHarris360 added the P1 - important Priority: High impact on UX label Apr 16, 2025
@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Apr 16, 2025

.alert.warning {
position: sticky;
z-index: 1;
Copy link
Member

Choose a reason for hiding this comment

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

I know this was not introduced by this PR, but since we changed the cards to use KCard, cards thumbnails now have a z-index: 1, and this warning is appearing behind these thumbnails. Could you increase it to two now that we are modifying this part of the code? 😅

image

Although I dont know if this is something we should fix in 0.18.0 cc: @rtibbles.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing this out! Since it wouldn't introduce any styling conflicts in this current PR, I went ahead and updated the z-index to 2.

@rtibbles rtibbles changed the base branch from develop to release-v0.18.0 April 17, 2025 15:31
@rtibbles rtibbles added P0 - critical Priority: Release blocker or regression and removed P1 - important Priority: High impact on UX labels Apr 17, 2025
@radinamatic
Copy link
Member

radinamatic commented Apr 17, 2025

Manual question selection banner is sticky no more! 💥

Chrome Firefox
https://github.com/user-attachments/assets/2f27eb7d-56a3-4258-b9dd-a9e397d871e1 https://github.com/user-attachments/assets/79c1313c-9e66-49ab-9748-d58f5e867592

Confirmed also when browsing bookmarks 👍🏽

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

Manual QA passes, good to go! 💯 :shipit:

@rtibbles rtibbles merged commit d6494c5 into learningequality:release-v0.18.0 Apr 17, 2025
51 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.) DEV: frontend P0 - critical Priority: Release blocker or regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manual question banner does not need to be sticky
4 participants