Skip to content

Private/rparth07/comment usability via keys#15110

Open
rparth07 wants to merge 3 commits intomainfrom
private/rparth07/comment-usability-via-keys
Open

Private/rparth07/comment usability via keys#15110
rparth07 wants to merge 3 commits intomainfrom
private/rparth07/comment-usability-via-keys

Conversation

@rparth07
Copy link
Contributor

Changes:

  1. enhance comment navigation by managing focus state
    • issue: there was no way we can reach existing comment from navigator -> Comments section using keyboard only
    • this changes fixes this issue by moving the focus on active comment's edit button so that user can modify it via keyboard
  2. a11y: enhance comment editing usability by adding keyboard support
  3. a11y: prevent unwanted focus on comment node
    • issue: comment node was taking focus while navigating inside comment using tab key.
    • Adding tabindex=-1 prevents moving focus via tab

Checklist

  • I have run make prettier-write and formatted the code.
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

- issue: there was no way we can reach existing comment from navigator -> Comments  section using keyboard only

- this changes fixes this issue by moving the focus on active comment's edit button so that user can modify it via keyboard

Signed-off-by: Parth Raiyani <parth.raiyani@collabora.com>
Change-Id: I104e653d5b742be2a8d7b55af73cdbe0fcb03b60
Signed-off-by: Parth Raiyani <parth.raiyani@collabora.com>
Change-Id: Ic748903d8c490bf302730a8d42f90bccd7533bba
issue: comment node was taking focus while navigating inside comment using tab key.

- Adding tabindex=-1 prevents moving focus via tab

Signed-off-by: Parth Raiyani <parth.raiyani@collabora.com>
Change-Id: Ia7bd9c927e5a3849f643b3b3a5aa07451e72dc7e
private editOnKeyPress (e: any): void {
if (e.code === 'Space' || e.code === 'Enter')
this.onEditComment(e);
window.L.DomEvent.stopPropagation(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to stop propagation for all key presses, or just the space/enter ones?

var commentSection = app.sectionContainer?.getSectionWithName(
app.CSections.CommentList.name,
) as cool.CommentSection;
if (commentSection) commentSection.pendingNavigatorFocus = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

So if a comment list section exists, which presumably is true if there are any comments, then if any treeview entry is activated this flag is set. So the comment section pendingNavigatorFocus would get set to true even if a non-comment is activated?

What's the callstack when the navigator triggered comment activation happens? Is it via the select method of CommentListSection.ts ? Could we put it in that select or something like that in response to (presumably core) telling us that the comment is to be activated rather than trying to predict what will happen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Review

Development

Successfully merging this pull request may close these issues.

2 participants