Skip to content

[a11y] We don't mark the disabled objects as disabled #7322

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
Closed
1 of 10 tasks
unlikelyzero opened this issue Dec 21, 2023 · 4 comments · Fixed by #7342
Closed
1 of 10 tasks

[a11y] We don't mark the disabled objects as disabled #7322

unlikelyzero opened this issue Dec 21, 2023 · 4 comments · Fixed by #7342
Labels
type:bug verified Tested or intentionally closed
Milestone
Target:4.0.0

Comments

@unlikelyzero
Copy link
Contributor

unlikelyzero commented Dec 21, 2023 •
edited
Loading

Summary

The a11y tests picked up that we're not marking disabled objects with the html semantic for disabled. We're just marking as !important .

According to https://sean-elliott.medium.com/a11y-tips-disabled-buttons-and-colour-contrast-f8824d5e9610, the a11y tooling will detect when an element is disabled and ignore color contrast violations. So this should be as simple as marking disabled objects as disabled with html semantics.

Known offenders:

  • Create Button
  • Edited Object in Tree
  • Dropdown in Fault Management Select Options

Environment

  • Open MCT Version: 3.3.0

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?

Additional Information

Screenshot 2023-12-20 at 9 36 00 PM
@ozyx ozyx self-assigned this Dec 21, 2023
@unlikelyzero unlikelyzero changed the title [a11y] We don't mark the disabled objects as disabled. Instead we mark !important [a11y] We don't mark the disabled objects as disabled Jan 2, 2024
charlesh88 added a commit that referenced this issue Jan 4, 2024
@charlesh88
Closes #7322
0a23ad6 
- New CSS for `aria-disabled = true` property.
- Changed multiple items to use aria-disabled instead of .disabled, including:
  - Action menu
  - Super menu
  - Notebook drag area
- Tree item style modded to only italicize when is-navigated and is being edited.
charlesh88 added a commit that referenced this issue Jan 4, 2024
@charlesh88
Closes #7322
3aa2f13 
- New CSS for `aria-disabled = true` property.
- Changed multiple items to use aria-disabled instead of .disabled, including:
  - Action menu
  - Super menu
  - Notebook drag area
- Tree item style modded to only italicize when is-navigated and is being edited.
- Create button sets itself to `disabled` when the editor is in use.
charlesh88 added a commit that referenced this issue Jan 4, 2024
@charlesh88
Closes #7322
0299998 
- Create button now _actually_ sets itself to `aria-disabled` when the editor is in use.
- CSS removes selector for `.is-editing`.
@charlesh88
Copy link
Contributor

charlesh88 commented Jan 5, 2024 •
edited
Loading

Testing Notes

Action menu:

  1. Go to a Telemetry Table with some data. Ensure that no rows are selected.
  2. Invoke the Action menu. Verify that "Export Marked Rows" and "Unmark All Rows" appear disabled and cannot be clicked.
    Screenshot 2024-01-04 at 3 36 45 PM
  3. Inspect the menu with Dev Tools. On the Elements tab, verify that each <li> element is using aria-disabled = "true" and that the class definition doesn't include "disabled".
    Screenshot 2024-01-04 at 3 42 35 PM
  4. Select some rows in the table. Repeat step 2, but verify that the previously disabled menu items are now enabled and can be clicked.
    Screenshot 2024-01-04 at 3 37 33 PM

Create button:

  1. Edit any editable view.
  2. Verify that the Create button appears disabled and can't be clicked.
  3. Inspect the button with Dev Tools. On the Elements tab, verify that the button is using aria-disabled = "true" and that the class definition doesn't include "disabled".
    Screenshot 2024-01-04 at 4 00 20 PM
  4. Exit Edit mode. Verify the button appears enabled and can be clicked.

Tree items

NOTE: a purely stylistic change was made here to improve contrast of tree items currently being edited; a disabled property is not being used.

  1. Navigate to an editable view, then edit it.
  2. In the tree, verify that the currently navved and editing tree item appears with italic text, but does NOT appear dimmed back.
    Screenshot 2024-01-04 at 4 05 43 PM

Notebook

  1. Navigate to a Notebook.
  2. Create a new entry or edit an existing entry.
  3. Verify that the drag area at the top of the Notebook appears disabled and can't be clicked.
    Screenshot 2024-01-04 at 4 22 10 PM

@charlesh88 charlesh88 mentioned this issue Jan 5, 2024
Merged
15 tasks
@unlikelyzero unlikelyzero added this to the Target:4.0.0 milestone Jan 17, 2024
unlikelyzero added a commit that referenced this issue Jan 26, 2024
@charlesh88 @unlikelyzero
Set disabled items to use disabled property (#7342)
1fc6056 
* Closes #7322
- New CSS for `aria-disabled = true` property.
- Changed multiple items to use aria-disabled instead of .disabled, including:
  - Action menu
  - Super menu
  - Notebook drag area
- Tree item style modded to only italicize when is-navigated and is being edited.

* Closes #7322
- New CSS for `aria-disabled = true` property.
- Changed multiple items to use aria-disabled instead of .disabled, including:
  - Action menu
  - Super menu
  - Notebook drag area
- Tree item style modded to only italicize when is-navigated and is being edited.
- Create button sets itself to `disabled` when the editor is in use.

* Closes #7322
- Create button now _actually_ sets itself to `aria-disabled` when the editor is in use.
- CSS removes selector for `.is-editing`.

* fix conflict

---------

Co-authored-by: John Hill <[email protected]>
@jvigliotta
Copy link
Contributor

Verified Fixed in Testathon 1/31/24

@rukmini-bose
Copy link
Contributor

Verified Testathon 2/1/24.

@ozyx
Copy link
Contributor

ozyx commented Feb 5, 2024

Verified Testathon 2/5/2024

Clicking on disabled menu items still closes the menu, though. We should change that

@ozyx ozyx added the verified Tested or intentionally closed label Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type:bug verified Tested or intentionally closed
Projects
None yet
Milestone
Target:4.0.0
Development

Successfully merging a pull request may close this issue.

Set disabled items to use disabled property nasa/openmct
5 participants
@charlesh88 @ozyx @jvigliotta @unlikelyzero @rukmini-bose