NEXT: Svelte Tabs Component

[Mahmoud-zino]

Linked Issue

Closes #2395

Description

tabs-next

[changeset-bot]

⚠️ No Changeset found

Latest commit: 82d99eb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
skeleton-docs	❌ Failed (Inspect)			Mar 28, 2024 10:02pm
skeleton-themes	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 28, 2024 10:02pm

[vercel]

Someone is attempting to deploy a commit to the Skeleton Labs Team on Vercel.

A member of the Team first needs to authorize it.

[endigo9740]

@Mahmoud-zino this is great, but since the feature is not added to the main doc site it won't generate a preview for it. So I can't check until I'm back to my workstation - likely on Monday. In the meantime can you help remind me to flush out the tickets before we start implementing features?

• NEXT: Tabs #2395

This issue is empty, and that should almost never be the case before we begin a feature.

I realize you've got time to get a head start and that's great, but I want to avoid a situation where there's a major change not documented that doesn't get missed. To avoid a lot of duplicated effort or even requiring a feature to be rebuilt from the ground up. Plain and simple, I don't want to waste your time!

There's a few things for Tabs specifically I need to put some thought into:

• If we want to stick with the TabGroup/Tab naming scheme. TabAnchor came in later, and to some degree it feels a bit clunky.
• The default styling. I'm working with Bohdan on the Figma side to try and get ahead of us so we have some references soon
• Any other notable tweaks or feature additions or changes

Figma stuff aside, I'll try to get the rest asap.

[endigo9740]

@Mahmoud-zino I've had a moment to run through and write out my proposed updates and changes for the Tabs component feature:

• NEXT: Tabs #2395

Let me know if you have any specific questions or feedback regarding these.

Just FYI I'm going to try to devote some time this afternoon to flushing this out for a few other upcoming components so this is ready before you start next time (sorry about that!).

Likewise I'm going to ping Bohdan to see if he can work up a mock for the tab style for us that you can follow. If you need help with with design, I don't mind jumping in as well!

[endigo9740]

@Mahmoud-zino I've gone ahead and closed all feedback comments above so we can start "fresh". You were right, there's a number of places where the ARIA spec diverges from my expectations, so we'll defer to their instruction.

I've made a number of small to medium sized changes, which you can review here:
46707b6

Preview Page

• Created variables for the icons
• Set the icons to 16px to match Figma
• Renamed the panel() snippet to panels() to be more semantic
• Added an "Icons Only" set to preview

Design

NOTE: this just details more "sweeping" changes. There's lots of micro adjustments via default prop values.

• I need to communicate with Bohdan to explain how Tailwind treats focus/outline styles. FYI for now we'll be off-spec.
• I've changed the default Tab width to w-full versus w-fit.
• I'm now using Tailwind's group modifier to affect the hover state. Now uses a tonal preset to match Figma.

Components

General

• Swapped from space properties to gap. This better prepares us for a vertical layout.

Tabs (root)

• Removed the wrapping div around tabpanels, which is not part of the ARIA example. The role and tabindex have been moved to the panel component. I also removed the $effect() logic, which introduces an issue I'll talk about below.
• Renamed snippet panel -> panels to be more semantic that this contains multiple items

Tabs.Control

• Removed nearly all classes from the first label > div. This element should be considered structural, not cosmetic. All styles have been redistributed the root label or the "content" element within. This greatly simplifies the number of style props users have to deal with.
• Modifying a number of default styles to better match the Figma docs. This includes using a preset-tonal style for group-hover, removing the outline styles (for now), and adding a missing base class for one element.
• I've updated the onKeyDown handler to strip out the space/enter features - which are never possible to use
• Updated the onKeyDown handler to document each portion of the process. There's a lot going on here, and there may still be room for further optimization

Tabs.Panel

• Moved the role and tabindex attributes here to better match the ARIA example.
• Added an optional id prop to match the ARAIA example

Tab Panel Tabindex Change

Per the panel change - I know you implemented the extra wrapping div to handle the auto-selection between the panel and panel contents (when a focusable element is present). We can likely migrate this logic from the Tabs -> Panel component. I unfortunately have run out of time to do this today, but will plan to pick up here tomorrow (sorry!)

Next Steps

I'm like 95% happy with the state of this now, but I want to revisit tomorrow to try and simplify the styles a bit more. I think we can find a happy medium between the default styles and ease of use for customization. Let's plan to sync tomorrow on what remains.

Btw it dawned on me that I never showed you guys how to use the dev-mode for Figma, which can be invaluable for checking what sizes/colors/etc are used in Figma, so you can more closely match the design. I'll aim to record a video asap to showcase how this works! (SENT ON DISCORD)

But overall great job bud!

[endigo9740]

@Mahmoud-zino here's the second wave of updates:

• Further refinement of the local SvelteKit preview page
• Updated the TW config to enable all themes for either preview app
• Verified the tab styles in light/dark mode and for all themes
• Removed the built-in overflow-x (see discussion below)
• Adjusted and rearranged the various props, especially for Tab.Control
• Implemented optional ID props in case folks need specific selectors for E2E tests.

Per overflow-x, once again this may have some impact on an (eventual) vertical mode, assuming we go down that path. It also negatively affects the outline for tab focus. I find it to be an anti-pattern in that if you have so many tabs they won't fit on your UI, you probably have too many tabs! Using responsive styling to switch to icons is typically better too. That said, I think we have two options here:

1. Lean into the table-wrapper approach, where this uses an external scroll element
2. Look into something like: https://ui.shadcn.com/docs/components/scroll-area

You now have my blessing to port this to React. Likewise once Hugo's doc updates are complete, let's make sure to return and add this component to the doc site! Given this PR updates several portions of the doc site - I'd advise we merge this BEFORE the doc updates, so keep the merge simpler on this end.

[endigo9740]

@Mahmoud-zino I've go ahead and merged this PR. The reason being is this had some linting changes that spilled over into the documentation section. We need the doc section as "clean" as possible before #2561 is merged. Otherwise we're in for some nasty merge conflicts.

I'm going to rename this PR to make it specific to the Svelte component. Feel free to start a follow up PR to port the React version of the Tabs component next. By doing this (assuming you start after #2561 is merged) you'll actually be able to implement both the Svelte and React components in the docs!

Feel free to check with me or Hugo if you need additional details around the doc updates.