Skip to content

Add focus for a valid element in lists support - #13

Merged
ai merged 21 commits into
ai:mainfrom
ignatiqq:list-focus
Mar 4, 2024
Merged

Add focus for a valid element in lists support#13
ai merged 21 commits into
ai:mainfrom
ignatiqq:list-focus

Conversation

@ignatiqq

@ignatiqq ignatiqq commented Mar 3, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

solves #11

@ignatiqq
ignatiqq marked this pull request as draft March 3, 2024 12:27
@ai

ai commented Mar 3, 2024

Copy link
Copy Markdown
Owner

Can you explain the use case?

@ai

ai commented Mar 3, 2024

Copy link
Copy Markdown
Owner

If you are talking about #11 task, note that it introduce custom data- attributes. This is why you should not use tagName.

@ignatiqq
ignatiqq marked this pull request as ready for review March 3, 2024 15:02
@ignatiqq
ignatiqq marked this pull request as draft March 3, 2024 15:05
@ignatiqq

ignatiqq commented Mar 3, 2024

Copy link
Copy Markdown
Contributor Author

I'm sorry. I need help. If we focused on element which is disabled. But we have element which has both valid attrs:
'data-keyux-ignore-hotkeys'
'data-keyux-hotkeys'

should we click on it. Or we should only take care of activeElement "data-keyux-hotkeys"?

@ai

ai commented Mar 3, 2024 •
edited
Loading

Copy link
Copy Markdown
Owner

Can you write me to https://t.me/sitnik ? I think the chat will be better to explain the idea.

@ignatiqq
ignatiqq marked this pull request as ready for review March 3, 2024 19:57
ai
ai reviewed Mar 3, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 3, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 3, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 3, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/demo/index.tsx Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/demo/index.tsx Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js Outdated
if (attr) {
let el = where.querySelector(`#${attr}`)

if (el.getAttribute('aria-keyshortcuts') === (codeOverride || code)) {

@ai ai Mar 4, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We can remove this test. Buttons should be inside data-keyux-hotkeys pannel.

@ignatiqq ignatiqq Mar 4, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread hotkey.js Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/demo/index.tsx Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/demo/index.tsx Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/demo/index.tsx Outdated
</h2>
<ul>
<li data-keyux-hotkeys="panel" data-keyux-ignore-hotkeys tabIndex={0}>
<button>First button</button>

@ai ai Mar 4, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
<button>First button</button>
First item

We don’t need buttons here

@ignatiqq ignatiqq Mar 4, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/demo/index.tsx Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated

press(window, { key: 'v' })
equal(clicked, 'v2')
;(window.document.querySelector('ul li') as HTMLLIElement).focus()

@ai ai Mar 4, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
;(window.document.querySelector('ul li') as HTMLLIElement).focus()
window.document.querySelector<HTMLLIElement>('ul li').focus()

@ignatiqq ignatiqq Mar 4, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated
ai
ai reviewed Mar 4, 2024
View reviewed changes
Comment thread test/hotkey.test.ts Outdated
@ai
ai merged commit 689ace3 into ai:main Mar 4, 2024
@ai ai mentioned this pull request Mar 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ai ai ai left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ignatiqq @ai