Skip to content

Conversation

@shyakadavis
Copy link
Contributor

@shyakadavis shyakadavis commented Mar 25, 2025

  • Label
  • Menubar
  • Pagination
    Pagination.{Link, NextButton, PrevButton} have these errors
     Expression produces a union type that is too complex to represent.
    
    Removing Fallback in children={children || Fallback} fixes, makes you wonder how/why Fallback is causing this.
  • Progress
  • Radio Group
  • Resizable
  • Scroll Area
  • Select
  • Separator
  • Sheet
  • Skeleton
  • Slider
  • Switch
  • Table
  • Tabs
  • Textarea
  • Toggle
  • Toggle Group
  • Tooltip

Will add these in a separate P.R

  • Popover
  • Range Calendar
  • Sidebar
  • Sonner

@changeset-bot
Copy link

changeset-bot bot commented Mar 25, 2025

⚠️ No Changeset found

Latest commit: ca6fb6e

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2025

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
shadcn-svelte ✅ Ready (View Log) Visit Preview ca6fb6e

@huntabyte
Copy link
Owner

Is this good for review @shyakadavis ?

@shyakadavis
Copy link
Contributor Author

Hey, @huntabyte . I would say "yes", but would appreciate some guidance and/or eyes on these remaining issues:

  • Resizable: The horizontal resize bar in not visible
  • These won't accept data-slots:
    • Select.Root
    • Sheet.Root
    • Sheet.Portal
    • Tooltip.Root
    • Tooltip.Provider
  • Tooltip.Arrow is not rendering as intended.

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

These won't accept data-slots:
Select.Root
Sheet.Root
Sheet.Portal
Tooltip.Root
Tooltip.Provider

I think we had agreed these weren't necessary since they technically aren't part of the DOM.

As for the other 2 I am taking a look right now...

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

CleanShot 2025-03-31 at 08 12 20

🤔

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

CleanShot.2025-03-31.at.09.09.32.mp4

This may need some tuning but I was able to do this:

<script lang="ts">
	import { Tooltip as TooltipPrimitive } from "bits-ui";
	import { cn } from "$lib/utils.js";

	let {
		ref = $bindable(null),
		class: className,
		sideOffset = 0,
		side,
		children,
		...restProps
	}: TooltipPrimitive.ContentProps = $props();
</script>

<TooltipPrimitive.Portal>
	<TooltipPrimitive.Content
		bind:ref
		data-slot="tooltip-content"
		{sideOffset}
		{side}
		class={cn(
			"bg-primary text-primary-foreground animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-(--bits-tooltip-content-transform-origin) z-50 w-fit text-balance rounded-md px-3 py-1.5 text-xs",
			className
		)}
		{...restProps}
	>
		{@render children?.()}
		<TooltipPrimitive.Arrow>
			{#snippet child({ props })}
				<div
					{...props}
					class={cn(
						"bg-primary top z-50 size-2.5 rotate-45 rounded-[2px]",
						side === "top" && "translate-x-1/2 translate-y-[calc(-50%_+_2px)]",
						side === "bottom" && "-translate-x-1/2 -translate-y-[calc(-50%_+_1px)]",
						side === "right" && "translate-x-[calc(50%_+_2px)] translate-y-1/2",
						side === "left" && "-translate-y-[calc(50%_-_3px)]"
					)}
				></div>
			{/snippet}
		</TooltipPrimitive.Arrow>
	</TooltipPrimitive.Content>
</TooltipPrimitive.Portal>

The default svg icon rendered by bits was giving me some trouble but rendering a div seems to work fine.

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

The resizable thing is because you (I assume) copied the shadcn styles but forgot to correct data-pane-group-direction to the data-direction that pane forge uses. Just a simple find and replace 😊

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

Also we should probably mend the styling for tabs but idk if thats a priority for this PR
CleanShot 2025-03-31 at 09 20 14

@shyakadavis
Copy link
Contributor Author

Hey, let me see if I can address the remaining items now. Thanks, @ieedan

@shyakadavis
Copy link
Contributor Author

Screenshot 2025-03-31 at 22 29 54

🤔 Yeah, this is wrong.

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

that's weird it wasn't doing that for me

@shyakadavis
Copy link
Contributor Author

that's weird it wasn't doing that for me

I noticed that it only occurs when it's alone. When other tooltips are beside it, it's fine. 🤷

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

🤦‍♂️

Yeah what the heck man. It should be absolute so idk why that makes a difference...

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

CleanShot.2025-03-31.at.15.57.53.mp4

Doesn't seem to be taking the styles...

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

I am so dumb but I found it. The side prop needs a default value.

let {
	ref = $bindable(null),
	class: className,
	sideOffset = 0,
+	side = "top",
	children,
	...restProps
}: TooltipPrimitive.ContentProps = $props();

@shyakadavis
Copy link
Contributor Author

As for the tabs, I can push changes, but my question is, what look are we aiming for? The one on ui.shadcn.com, or the one v4.shadcn.com?

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

I think we just need to override the styles for the original. It should still look like v4 normally but for the component preview and stuff it should look like before.

@huntabyte
Copy link
Owner

I think it's fine for now tbh @ieedan - we can worry about that before the final merge depending on what shad does with his docs!

@ieedan
Copy link
Collaborator

ieedan commented Mar 31, 2025

In that case LGTM!

Copy link
Owner

@huntabyte huntabyte left a comment

Choose a reason for hiding this comment

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

You're awesome!

@huntabyte huntabyte merged commit a615c82 into huntabyte:next-tailwind-4 Apr 1, 2025
6 checks passed
@shyakadavis shyakadavis deleted the chore/tv-4 branch April 1, 2025 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants