feat: add option to make TabList button be a element #3508
Conversation
omBratteng
requested review from
rebelchris,
ilasw,
capJavert,
sshanzel and
nensidosari
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
omBratteng
force-pushed
the
AS-543-buttons-to-links
branch
from
becb607 to
d7166a6
Compare
| @@ -113,7 +118,7 @@ function TabList<T extends string = string>({ | |||
| ); | |||
|
|
|||
| return ( | |||
| <button | |||
| <Tag | |||
There was a problem hiding this comment.
If now is an anchor maybe we want to add useful attributes like aria-label, title, href (this should be required imo). What do you think?
There was a problem hiding this comment.
I've added label and title. Adding href needs a bit more work I see
There was a problem hiding this comment.
If it's hard to implement, also we can put role="button" and href="#" if the behaviour is fully JS-side
There was a problem hiding this comment.
Had a chat with @rebelchris, we'll merge it without the href and then run it through the SEO tool to look. If it still complains, I'll refactor it so that it gets proper href.
| isActive ? '' : 'text-text-tertiary', | ||
| isAnchor && 'cursor-pointer', |
There was a problem hiding this comment.
Non mandatory: if we have more conditional classes maybe the object structure can be more readable, wdyt?
classNames(
className.item,
'relative p-2 py-4 text-center font-bold typo-callout',
{ 'text-text-tertiary': !isActive, 'cursor-pointer': isAnchor},
)There was a problem hiding this comment.
Don't think we'll have any more now
| @@ -126,13 +131,14 @@ function TabList<T extends string = string>({ | |||
| className.item, | |||
| 'relative p-2 py-4 text-center font-bold typo-callout', | |||
| isActive ? '' : 'text-text-tertiary', | |||
| isAnchor && 'cursor-pointer', | |||
| )} | |||
| onClick={() => onClick(tab)} | |||
| type="button" | |||
There was a problem hiding this comment.
If anchor type button should not be there
omBratteng
force-pushed
the
AS-543-buttons-to-links
branch
from
fb45c4e to
6fa54d5
Compare
Changes
ainstead ofbuttonPreview domain
https://as-543-buttons-to-links.preview.app.daily.dev