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.

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

Allow default input behavior for text selection on TagsInput and Combobox #1318

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/radix-vue/src/Combobox/ComboboxInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ function handleKeyDown(ev: KeyboardEvent) {
}

function handleHomeEnd(ev: KeyboardEvent) {
// allow text selection while maintaining `shift` key
if(ev.shiftKey)
return

ev.preventDefault()

if (!rootContext.open.value)
return
rootContext.onInputNavigation(ev.key === 'Home' ? 'home' : 'end')
Expand Down Expand Up @@ -87,7 +93,7 @@ function handleInput(event: Event) {
@input="handleInput"
@keydown.down.up.prevent="handleKeyDown"
@keydown.enter="rootContext.onInputEnter"
@keydown.home.end.prevent="handleHomeEnd"
@keydown.home.end="handleHomeEnd"
>
<slot />
</Primitive>
Expand Down
38 changes: 38 additions & 0 deletions packages/radix-vue/src/TagsInput/TagsInput.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ describe('given default TagsInput', () => {
expect(tags[tags.length - 2].attributes('data-state')).toBe('active')
})

it('should not select the previous tag when press Shift + ArrowLeft', async () => {
await input.trigger('keydown', {
key: 'ArrowLeft',
shiftKey: true,
})
expect(tags[tags.length - 1].attributes('data-state')).not.toBe('inactive')
expect(tags[tags.length - 2].attributes('data-state')).not.toBe('active')
})

it('should select the first item when press Home', async () => {
await input.trigger('keydown', {
key: 'Home',
Expand All @@ -80,6 +89,15 @@ describe('given default TagsInput', () => {
expect(tags[tags.length - 1].attributes('data-state')).toBe('inactive')
})

it('should not select the first item when press Shift + Home', async () => {
await input.trigger('keydown', {
key: 'Home',
shiftKey: true,
})
expect(tags[0].attributes('data-state')).not.toBe('active');
expect(tags[tags.length - 1].attributes('data-state')).not.toBe('inactive')
})

it('should select the last item when press End', async () => {
await input.trigger('keydown', {
key: 'Home',
Expand All @@ -91,13 +109,33 @@ describe('given default TagsInput', () => {
expect(tags[tags.length - 1].attributes('data-state')).toBe('active')
})

it('should not select the last item when press Shift + End', async () => {
await input.trigger('keydown', {
key: 'Home',
})
await input.trigger('keydown', {
key: 'End',
shiftKey: true,
})
expect(tags[0].attributes('data-state')).not.toBe('inactive');
expect(tags[tags.length - 1].attributes('data-state')).not.toBe('active');
})

it('should remove active state when press ArrowRight', async () => {
await input.trigger('keydown', {
key: 'ArrowRight',
})
expect(tags[tags.length - 1].attributes('data-state')).toBe('inactive')
})

it('should not remove active state when press Shift + ArrowRight', async () => {
await input.trigger('keydown', {
key: 'ArrowRight',
shiftKey: true,
})
expect(tags[tags.length - 1].attributes('data-state')).not.toBe('inactive')
})

describe('after pressing on Backspace', () => {
let prevTag: DOMWrapper<HTMLElement>
beforeEach(async () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/radix-vue/src/TagsInput/TagsInputRoot.vue
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ provideTagsInputRootContext({
case 'End':
case 'ArrowRight':
case 'ArrowLeft': {
// allow text selection while maintaining `shift` key
if(event.shiftKey)
break

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 don't need it for Tags Input. The current implementation works well, where we only focus on TagsInputItem when we are not highlighting any text

Screen.Recording.2024-10-01.at.9.30.46.PM.mov

Copy link
Author

Choose a reason for hiding this comment

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

I believe you used arrows to select char one by one. This one enables text selection in its entirety or what's left of it with shift+home or shift+end. That is the expected behaviour when using a text input.

This comment was marked as outdated.

Copy link
Author

Choose a reason for hiding this comment

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

What about the following behavior ? I removed the tests I wrote because I think they're not great and couldn't figure it out.

Screencast.from.2024-10-02.21-52-00.webm
Screencast.from.2024-10-02.21-55-07.webm

const isArrowRight = (event.key === 'ArrowRight' && dir.value === 'ltr') || (event.key === 'ArrowLeft' && dir.value === 'rtl')
const isArrowLeft = !isArrowRight
// only focus on tags when cursor is at the first position
Expand Down
Loading