-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix(ktabs): slotted anchor link #2480
Conversation
✅ Deploy Preview for kongponents-sandbox ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kongponents ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
<template> | ||
<KTableView | ||
row-key="id" | ||
:data="tableData" | ||
:headers="headers" | ||
/> | ||
</template> | ||
|
||
<script setup lang="ts"> | ||
import type { TableViewData } from '@kong/kongponents' | ||
|
||
const tableData: TableViewData = [ | ||
{ | ||
id: 'a70642b3-20f2-4658-b459-fe1cbcf9e315', | ||
... | ||
}, | ||
{ | ||
id: '58c599a9-f453-41f3-9e64-0a7fc6caedad', | ||
... | ||
} | ||
] | ||
</script> |
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.
Just making an example easier to understand
src/components/KTabs/KTabs.vue
Outdated
type: String, | ||
default: '0', | ||
validator: (val: string): boolean => val === '0' || val === '-1', |
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.
question: tabindex is always a number (even though it's passed as a string), why is this typed as a string? I think it might be easier to validate the provided value if the prop itself was a number.
e.g. if the user passes my-wrong-value
as a string, the validator will error; however, you'd still be passing the invalid value directly to the HTML. I think this should be prevented.
Also, I'm not sure the validator makes sense either given the description of the prop. The logic of the validator prevents me from passing a tabindex of any value other than 0
or -1
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.
Addressed. Added a check to make sure the passed value is either 0
or -1
and made the type more strict
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.
why can the value only be 0
or -1
? if we have a prop to customize the tabindex, why can't we allow any value?
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.
I forgot it can be other values 😅
Fixed now, accepts value -1 <= x <= 32767
(mdn)
Co-authored-by: Adam DeHaven <2229946+adamdehaven@users.noreply.github.com>
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.
lgtm
## [9.13.1](v9.13.0...v9.13.1) (2024-10-25) ### Bug Fixes * **ktabs:** slotted anchor link ([#2480](#2480)) ([72ae295](72ae295))
🎉 This PR is included in version 9.13.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
The issue:
When a link is slotted into anchor slot of a tab, when using keyboard nav, the user needs to press tab twice to navigate to next element (first time the wrapper element
div.tab-link
has focus, then the slotted link element).Solution:
Add
anchorTabindex
prop that allows set tabindex fordiv.tab-link
element. This allows to make only one element in tab focusable.Screen.Recording.2024-10-25.at.11.24.24.AM.mov