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.

Sign up for GitHub

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

Jump to bottom

[#12081] User-friendliness: Fix custom visibility options #12209

Merged
merged 8 commits into from
Mar 17, 2023
Merged

[#12081] User-friendliness: Fix custom visibility options #12209

merged 8 commits into from
Mar 17, 2023

Conversation

weiquu
Copy link
Contributor

@weiquu weiquu commented Mar 15, 2023 •
edited
Loading

Part of #12081
Sub-issue: Fix custom visibility options

Outline of Solution
Added aria-labels to the table and checkboxes, and changed the dropdown divider to aria-hidden. Also changed the dropdown items to be buttons, which fixed the screen reader issue.

The getCheckboxAriaLabel function looks rather inelegant to me. I'm considering moving most of the logic to 2 Map variables outside of the function... would that be better?
Edit: Thought about it and I think this improves understanding of the code, so I've made the change

@zhaojj2209 zhaojj2209 added the s.ToReview The PR is waiting for review(s) label Mar 16, 2023
jasonqiu212
jasonqiu212 reviewed Mar 16, 2023
View reviewed changes
Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

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

lgtm! Could you fix the lint issues please?

@weiquu
Copy link
Contributor Author

weiquu commented Mar 17, 2023

Fixed the E2E test issue! Thanks @zhaojj2209 for helping to debug (:

Quick summary of the issue: appears for FeedbackNumScaleQuestionE2ETest, but only for firefox (works fine on chrome). Somehow caused by the closing of the dropdown menu, which results in the step value of the num scale to be 0. Thus, the save button doesn't go stale due to the error when saving the question (step increment must be > 0).

zhaojj2209
zhaojj2209 approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes!

@zhaojj2209 zhaojj2209 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Mar 17, 2023
@zhaojj2209 zhaojj2209 merged commit ebb6693 into TEAMMATES:user-friendliness Mar 17, 2023
zhaojj2209 pushed a commit that referenced this pull request Mar 23, 2023
@weiquu @zhaojj2209
[#12081] User-friendliness: Fix custom visibility options (#12209)
e877ee6 
* Fix custom visibility options

* Lint and tests

* Add more labels and update tests

* Fix attempt for E2E tests

* Refine aria label function logic

* Fix lint issues

* E2E issues

* E2E fix
zhaojj2209 pushed a commit that referenced this pull request Mar 23, 2023
@weiquu @zhaojj2209
[#12081] User-friendliness: Fix custom visibility options (#12209)
ee27fb6 
* Fix custom visibility options

* Lint and tests

* Add more labels and update tests

* Fix attempt for E2E tests

* Refine aria label function logic

* Fix lint issues

* E2E issues

* E2E fix
zhaojj2209 pushed a commit that referenced this pull request Mar 25, 2023
@weiquu @zhaojj2209
[#12081] User-friendliness: Fix custom visibility options (#12209)
4d1d3c6 
* Fix custom visibility options

* Lint and tests

* Add more labels and update tests

* Fix attempt for E2E tests

* Refine aria label function logic

* Fix lint issues

* E2E issues

* E2E fix
zhaojj2209 pushed a commit that referenced this pull request Mar 25, 2023
@weiquu @zhaojj2209
[#12081] User-friendliness: Fix custom visibility options (#12209)
69ff000 
* Fix custom visibility options

* Lint and tests

* Add more labels and update tests

* Fix attempt for E2E tests

* Refine aria label function logic

* Fix lint issues

* E2E issues

* E2E fix
@zhaojj2209 zhaojj2209 added this to the V8.26.0 milestone Apr 2, 2023
@zhaojj2209 zhaojj2209 added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Apr 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonqiu212 jasonqiu212 jasonqiu212 left review comments

@zhaojj2209 zhaojj2209 zhaojj2209 approved these changes

Assignees

@jasonqiu212 jasonqiu212

@zhaojj2209 zhaojj2209

Labels
c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V8.26.0
Development

Successfully merging this pull request may close these issues.
3 participants
@weiquu @jasonqiu212 @zhaojj2209