Skip to content

Conversation

@vinicius73
Copy link
Member

Summary

Enable tab usage on session list popover.

  • Add visual feedback about focus
  • restore focus on editor after close popover
Peek.2022-08-01.19-38.mp4
Peek.2022-08-01.19-39.mp4

Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Codewise looks good to me... just one minor comment. I still approve so you can fix if you want and then go ahead without another review.

I did not try this myself - your screen capture seems to make sense.

<p class="hint">
{{ t('text', 'Author colors are only shown until everyone has closed the document.') }}
</p>
<form ref="form" tabindex="0" @submit.prevent>
Copy link
Member

Choose a reason for hiding this comment

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

I think we should rather put the initial focus on https://github.com/nextcloud/text/pull/2771/files#diff-9e0a2a342f91d6277778b1bb6b17da4f37cdab38ef53c98a7c97a84deaf488a5R40 so that the content gets read and maybe give this a descriptive label then

<Popover class="session-list"
placement="bottom"
@after-show="applyFocus"
@after-hide="revertFocus">
Copy link
Member

@juliusknorr juliusknorr Aug 2, 2022

Choose a reason for hiding this comment

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

Maybe we can additionally catch tab/escape keys and prevent the default event handling and instead just revert the focus.

this.$refs.form.focus()
},
revertFocus() {
this.$editor.chain().focus().run()
Copy link
Member

Choose a reason for hiding this comment

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

Should this rather go back to focus on the button after closing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know. But I am open for ideas.

@juliusknorr
Copy link
Member

@vinicius73 Any thoughts about using something like in nextcloud-libraries/nextcloud-vue#2654 ?

Could also be fixed upstream globally then nextcloud-libraries/nextcloud-vue#2860

@vinicius73
Copy link
Member Author

@vinicius73 Any thoughts about using something like in nextcloud/nextcloud-vue#2654 ?

Looks interesting, I will play around this a bit.

Vinicius Reis and others added 3 commits August 2, 2022 11:19
Co-authored-by: max-nextcloud <[email protected]>
Signed-off-by: Vinicius Reis <[email protected]>
Signed-off-by: Vinicius Reis <[email protected]>
@vinicius73
Copy link
Member Author

Works like a charm @juliushaertl
I will do some upstream of this, maybe we can drop this PR.

@vinicius73
Copy link
Member Author

Waiting for next release of @nextcloud/vue

nextcloud-libraries/nextcloud-vue#2941

@vinicius73 vinicius73 closed this Aug 10, 2022
@vinicius73 vinicius73 deleted the 2733-bad-focus-order-for-collaborator-menu branch August 10, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bad focus order for collaborator menu

4 participants