-
Notifications
You must be signed in to change notification settings - Fork 642
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
Show other authors editing the same element #13420
Merged
brandonkelly
merged 33 commits into
4.5
from
feature/dev-1154-show-other-authors-editing-the-same-element
Aug 4, 2023
Merged
Show other authors editing the same element #13420
brandonkelly
merged 33 commits into
4.5
from
feature/dev-1154-show-other-authors-editing-the-same-element
Aug 4, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DEV-1154 Show other authors editing the same element
When editing an element, other authors’ activity for the element should be reported. |
gcamacho079
requested changes
Jul 17, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Keyboard users can’t currently access author information. Instead of a
title
tooltip, it's recommended a keyboard-accessible toggletip is used instead (such as the one introduced in Use inclusive toggletip guidance for InfoIcon.js #12272 ).
In this case, a toggle button should be nested inside the list item, and should have an accessible name along the lines of "{{username}}, {{active/inactive}} author. More details." - All user images (SVG and user-uploaded) in the activity container the image can be considered decorative, since the accessible name should come from the toggletip button. You can hide both from the accessibility tree by setting
role=" presentation"
on the SVG andimg
. - For inactive users with no uploaded image, the color contrast of the user icon over its background (
--gray-050
) is 1.7:1. This should be modified to be at least 3:1. - The color contrast between active vs. inactive user icons (for users with no image uploaded to the CP) is 2:1. For users with a user image set, the change from inactive to active is a Use of Color issue, as the change of state is only communicated via a change in color.
Although users can also get that info by viewing the toggletip, it may make sense to implement another visual cue for the author state. (An example of this is the dot indicator that shows on Spotify when shuffle is active.) This would improve the UX because users who aren't able to differentiate the contrast change between states won't have to open each toggletip to see who's actively editing the element. - When there are enough authors to overflow the container, the contents cause horizontal viewport scrolling. Instead, authors should wrap onto additional lines to prevent horizontal scrolling or overflowing authors should move into a disclosure menu. (Tested on 320px viewport by duplicating
li
elements.) - Part of a general notification issue, but when there's a notice with a focusable element inside, we should keep the notice open until the user dismisses it or focus leaves the container, even if the user has a shorter notice duration set. (After another author edited the element, I received the notification on Safari/VO, but the notification closed before the notification readout finished, so I was unable to hit the reload button.)
-
Screen reader users do not get the same information that sighted users do about author activity via visible changes to the author list. Changes to the author activity container (such as toggling the faded class, or another author viewing the entry) should be accompanied by announcements via a live region, such as "Another author is editing this entry" or "Another author is viewing this entry."Because it's a bit noisy with other updates happening on the page, and it's not a true "status" message, I'm removing this from the issue list.
…he-same-element # Conflicts: # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map # src/web/assets/cp/dist/css/cp.css # src/web/assets/cp/dist/css/cp.css.map
[ci skip]
…enter and start setting in Editor js
@brandonkelly another issue I missed during the first run-through is that users should have the ability to pause/resume auto-updating regions to meet SC 2.2.2: Pause, Stop, Hide. |
…he-same-element # Conflicts: # src/config/app.php # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map # src/web/assets/cp/dist/css/cp.css # src/web/assets/cp/dist/css/cp.css.map # src/web/assets/garnish/dist/garnish.js # src/web/assets/garnish/dist/garnish.js.map
…ement' into a11y/live-region-and-roles # Conflicts: # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map # src/web/assets/cp/dist/css/cp.css # src/web/assets/cp/dist/css/cp.css.map
…he-same-element [ci skip]
…ement' into a11y/live-region-and-roles # Conflicts: # src/web/assets/pluginstore/dist/css/app.css.map # src/web/assets/pluginstore/dist/js/app.js # src/web/assets/pluginstore/dist/js/app.js.map
…he-same-element # Conflicts: # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map # src/web/assets/cp/dist/css/cp.css # src/web/assets/cp/dist/css/cp.css.map
…ement' into a11y/live-region-and-roles # Conflicts: # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map # src/web/assets/cp/dist/css/cp.css # src/web/assets/cp/dist/css/cp.css.map
…cms/cms into a11y/live-region-and-roles # Conflicts: # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map
Update toggletip element roles
…he-same-element # Conflicts: # src/web/assets/cp/dist/cp.js # src/web/assets/cp/dist/cp.js.map
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Updates full-page element editors to show other authors who are currently viewing/editing the same element.
The user photos have tooltips that explain what they were doing, e.g. “Crystal is viewing this entry.” or “Travis saved this entry at 10:35 AM.”
Additionally, full-page element editors now track whether the element being edited has been changed externally, either directly or upstream, and will start displaying a “This entry has been updated.” notice when that happens, with a “Reload” button.
Related issues