Skip to content
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

feat: customisable composable components #910

Open
1 task done
veloii opened this issue Aug 11, 2024 · 23 comments
Open
1 task done

feat: customisable composable components #910

veloii opened this issue Aug 11, 2024 · 23 comments
Labels
area:packages issue regarding one of the uploadthing packages ✨ enhancement suggestion or feature request to improve uploadthing PRs Accepted Anyone feel free to pick this up

Comments

@veloii
Copy link

veloii commented Aug 11, 2024

Describe the feature you'd like to request

I'd like to request unstyled, composable components for uploadthing:

  • Inline uploadable image component (e.g. for avatars)
  • Composable versions of existing file upload components

These would be bare-bones, allowing easy styling and flexible combinations. I've already had to build similar things for my own sites. This kind of stuff is gold for DX - saves time and headaches.

Describe the solution you'd like to see

I've already got an UploadableInlineImage component in my site at the moment. The other components would have a similar API.
All the components can be customised with your current styling solution, for the examples I'm using Tailwind.

Hierarchy:

<UploadableInlineImage>
	<UploadableInlineImageProgressOverlay />
	<UploadableInlineImageOverlay>
		<UploadableInlineImageEditButton />
		<UploadableInlineImageDeleteButton />
	</UploadableInlineImageOverlay>
	<UploadableInlineImageDropzone>
		<UploadableInlineImageContent  />
		<UploadableInlineImageEmpty />
	</UploadableInlineImageDropzone>
</UploadableInlineImage>

Example Usage:

<UploadableInlineImage
	endpoint="updateBanner"
	// all the props like onUploadError, onClientUploadComplete, etc could be here
	imageUrl={imageUrl}
	className="h-32 w-full md:h-auto md:rounded-xl outline outline-1 -outline-offset-1 outline-zinc-950/10 bg-white"
>
	<UploadableInlineImageProgressOverlay variant="bar" />

	<UploadableInlineImageDropzone className="transition data-[drag]:opacity-50">
		<UploadableInlineImageOverlay className="group-hover:opacity-100 opacity-0 transition w-full h-full bg-foreground/60 text-background border sm:rounded-xl flex flex-col gap-2 justify-center items-center">
			<Icons.media className="opacity-50" />
			<span>Select a photo</span>
		</UploadableInlineImageOverlay>

		<UploadableInlineImageContent
			draggable={false}
			className="object-cover"
			alt="Banner"
		/>

		<UploadableInlineImageEmpty>
			<div className="w-full h-full inline-flex flex-col gap-2 justify-center items-center">
				<Icons.media className="opacity-50" />
				<span>Select a photo</span>
			</div>
		</UploadableInlineImageEmpty>
	</UploadableInlineImageDropzone>
</UploadableInlineImage>

I'm not 100% sure on UploadableInlineImageProgressOverlay as that's currently not unstyled at the moment, but I think that could be worked out easily.

Additional information

No response

👨‍👧‍👦 Contributing

  • 🙋‍♂️ Yes, I'd be down to file a PR implementing this feature!
@veloii veloii added the ✨ enhancement suggestion or feature request to improve uploadthing label Aug 11, 2024
@markflorkowski markflorkowski added area:packages issue regarding one of the uploadthing packages PRs Accepted Anyone feel free to pick this up labels Aug 13, 2024
@markflorkowski
Copy link
Collaborator

@veloii This is definitely something we would be interested in, but internally do not have the bandwidth to build right now. If you are interested in putting together a PR, I am happy to review and provide feedback so that we can get something merged sooner!

@veloii
Copy link
Author

veloii commented Aug 14, 2024

Yes, I’d be happy to submit a PR, would you like a draft of the API before I start working on it?

@markflorkowski
Copy link
Collaborator

Yeah, that is a good idea, just to make sure we are on the same page before you put the effort into implementing.

@veloii
Copy link
Author

veloii commented Aug 16, 2024

Sorry for the delay - what about something like this?

Function signatures:

import React, { ElementType } from "react";
import type { ErrorMessage } from "@uploadthing/shared";
import type { FileRouter } from "uploadthing/types";

import type { UploadthingComponentProps } from "../types";
import { createUploadthing } from "uploadthing/server";

type PrimitiveComponentCallbackValues = {
  ready: boolean;
  isUploading: boolean;
  uploadProgress: number;
  fileTypes: string[];
};

type PrimitiveComponentProps<T extends ElementType = "div"> = Omit<
  React.ComponentPropsWithRef<T>,
  "children"
> & {
  children?:
  | ((values: PrimitiveComponentCallbackValues) => React.ReactNode)
  | React.ReactNode;
};

export type UploadPrimitiveProps<
  TRouter extends FileRouter,
  TEndpoint extends keyof TRouter,
  TSkipPolling extends boolean = false,
> = UploadthingComponentProps<TRouter, TEndpoint, TSkipPolling> &
  PrimitiveComponentProps;

declare function UploadPrimitive<
  TRouter extends FileRouter,
  TEndpoint extends keyof TRouter,
  TSkipPolling extends boolean = false,
>(
  props: FileRouter extends TRouter
    ? ErrorMessage<"You forgot to pass the generic">
    : UploadPrimitiveProps<TRouter, TEndpoint, TSkipPolling>,
): React.JSX.Element;

declare namespace UploadPrimitive {
  function Button(_: PrimitiveComponentProps<"button">): React.ReactNode;
  function AllowedContent(_: PrimitiveComponentProps): React.ReactNode;
  function ClearButton(_: PrimitiveComponentProps<"button">): React.ReactNode;
}

Demo:

const f = createUploadthing();

const router = {
  exampleRoute: f(["image"])
    .middleware(() => ({ foo: "bar" }))
    .onUploadComplete(({ metadata }) => {
      return { foo: "bar" as const };
    }),
};

type Router = typeof router;

function Demo() {
  return (
    <>
      <UploadPrimitive<Router, "exampleRoute">
        endpoint="exampleRoute"
        className="bg-blue-500"
      >
        <UploadPrimitive.Button className="ut-ready:bg-green-500">
          Choose Files(s)
        </UploadPrimitive.Button>

        <UploadPrimitive.ClearButton className="disabled:hidden">
          Clear files
        </UploadPrimitive.ClearButton>

        {/* Default content */}
        <UploadPrimitive.AllowedContent />

        {/* Custom content */}
        <UploadPrimitive.AllowedContent>
          {({ fileTypes }) => <div>Accepted: {fileTypes.join(", ")}</div>}
        </UploadPrimitive.AllowedContent>
      </UploadPrimitive>

      <UploadPrimitive<Router, "exampleRoute">
        endpoint="exampleRoute"
        className="bg-blue-500"
      >
        {/* get values here instead */}
        {({ fileTypes }) => (
          <>
            <UploadPrimitive.Button className="ut-ready:bg-green-500">
              Choose Files(s)
            </UploadPrimitive.Button>

            <UploadPrimitive.ClearButton className="disabled:hidden">
              Clear files
            </UploadPrimitive.ClearButton>

            <div>Accepted: {fileTypes.join(", ")}</div>
          </>
        )}
      </UploadPrimitive>
    </>
  );
}

PS: I'm aware namespace is an anti-pattern but it's the only way I can think of declaring a function on a declared function in TS

For the actual implementation, I'd just copy the current Upload button and create a context with all the PrimitiveComponentCallbackValues and have all the sub components read that.

Including @radix-ui/react-slot would allow the use of any component library (MUI, Mantine, etc) and is only 1.1kb minified+gzipped.

<UploadPrimitive.Button asChild>
  <YourComponentLibraryButton>
    Choose Files(s)
  </YourComponentLibraryButton>
</UploadPrimitive.Button>

I'm not 100% sure where this would go in the docs.

@veloii
Copy link
Author

veloii commented Aug 17, 2024

cc @markflorkowski

@markflorkowski
Copy link
Collaborator

Sorry for the delay, didn't have time over the weekend to review. Overall looks good to me, though I would probably rename the components themselves a bit:

import * as UT from ...

<UT.Root<Router, "exampleRoute">
  className="bg-blue-500"
>
  <UT.Trigger className="ut-ready:bg-green-500">
    Choose Files(s)
  </UT.Trigger>

  <UT.Clear className="disabled:hidden">
    Clear files
  </UT.Clear>

  {/* Default content */}
  <UT.AllowedContent />

  {/* Custom content */}
  <UT.AllowedContent>
    {({ fileTypes }) => <div>Accepted: {fileTypes.join(", ")}</div>}
  </UT.AllowedContent>
</UT.Root>;

I am ok with including @radix-ui/react-slot for the ergonomic win

cc @juliusmarminge -- thoughts?

As for where to put it in the docs, right now I would say as a sub-section on the theming page, but me may want to break that page into multiple, as it is already quite large 🤔

@juliusmarminge
Copy link
Collaborator

juliusmarminge commented Aug 19, 2024

I like the idea of unstyled very much.

API wise I've recently found HeadlessUI's <Parent as={Button}> prop approach a bit nicer to work with than Radix's Parent asChild><Button> but that's just personal preference. (Not sure if there's a package ready-to-use for this or not but feels like it should be fairly light weight to copy props?)

Same goes for render={({ types }) => UI} prop instead of <>{({ types }) => UI}</> children render function, but again that's personal preference and at the end of the day an implementation detail

@markflorkowski
Copy link
Collaborator

Same goes for render={({ types }) => UI} prop instead of <>{({ types }) => UI}</> children render function, but again that's personal preference and at the end of the day an implementation detail

You prefer render={({ types }) => UI} ?

@juliusmarminge
Copy link
Collaborator

Yea. idk I guess just prefer not indenting stuff unnecessarily 😅

@juliusmarminge
Copy link
Collaborator

Another thing I think we need is for files to be a controllable prop

@dieselftw
Copy link

Is this being worked on still? Would be a HUGE win imo. I have almost the exact same use case as OP -- except I just ended up making a messy workaround.

I'm willing to drop a PR over the weekend if no one's working on this.

@juliusmarminge
Copy link
Collaborator

Feel free!

One thing to note though is that #886 is touching quite a bit in the component code. We'll try and get that merged before EOW so you don't have to resolve a bunch of conflicts later on

@veloii
Copy link
Author

veloii commented Sep 4, 2024

Yeah sorry, I've been really busy and haven't found the time to work on a PR. I can help with some of the other primitive components after your initial PR if you like. For example, multiple file support and inline images / videos.

import * as UT from ...

<UT.Root<Router, "exampleRoute">
  className="bg-blue-500"
  multiple
>
  {({ files, removeFile }) => (
    <>
      <UT.Trigger className={files.length > 0 && "hidden"}>
        Choose Files(s)
      </UT.Trigger>
      
      <div>
        {files.map((file) => (
          <div>
            <span>{file.name}</span>
            <button onClick={() => removeFile(file.id)}>Remove</button>
          </div>
        ))}
      </div>

      <UT.Clear className="disabled:hidden">
        Clear files
      </UT.Clear>
    </>
  )}
</UT>;

or (this feels worst)

<UT.List>
  {({ files, removeFile }) => files.map((file) => (
    <div>
      <span>{file.name}</span>
      <button onClick={() => removeFile(file.id)}>Remove</button>
    </div>
  ))}
</UT.List>

Could also be done through a controllable prop, but for most use cases it's worst DX.

@juliusmarminge
Copy link
Collaborator

Could also be done through a controllable prop, but for most use cases it's worst DX.

Perhaps, but it's also gonna be more powerful and allow much more usages (state might come from parent, "updater" might be on a different part of the app outside of <UT.Root> etc etc

@veloii
Copy link
Author

veloii commented Sep 4, 2024

I completely agree. Sorry if I wasn't clear, I meant both controlled and non-controlled should be available but that I thought the DX is better if you aren't using it outside the component.

@veloii
Copy link
Author

veloii commented Sep 14, 2024

@juliusmarminge Working on a PR at the moment. How should I run my code locally to ensure it all works?

@juliusmarminge
Copy link
Collaborator

We don't have any UI tests atm so just spin up an example and test it manually. If you've made any logic changes to the internals our tests there are fairly comprehensive

@veloii
Copy link
Author

veloii commented Sep 14, 2024

Do you have an ideas on how the root component generics be generated?
This is what I've got at the moment.

import { generateUploadRoot } from "@uploadthing/react";
import * as UT from "@uploadthing/react/primitive";

const UploadRoot = generateUploadRoot<OurFileRouter>();

<UploadRoot>
  <UT.Button>
    Upload
  </UT.Button>
</UploadRoot>

but it's not very intuitive as we can't use UT.Root without passing the generics manually.
It also breaks the react context because the context is used in @uploadthing/react/primitive but the provider is created in @uploadthing/react for generateUploadRoot.

I thought about

import { generateUploadPrimitives } from "@uploadthing/react";

const UT = generateUploadPrimitives<OurFileRouter>();

<UT.Root>
  <UT.Button>
    Upload
  </UT.Button>
</UT.Root>

but I'm pretty sure this will break tree shaking as we define all the components as one variable.

Do you have any other ideas?

@veloii
Copy link
Author

veloii commented Sep 14, 2024

cc @juliusmarminge

@juliusmarminge
Copy link
Collaborator

How many component parts are there? Is tree shaking something we need to worry too much about? It's gonna tree shake if you're not using any of this custom component at all, so the only case where there'll be more JS than necessary is if you're using the root and button but not the rest of them?

Alternatively, we can do some svelte-y thing and have a function that returns the props

const typehelper = gen<UploadRouter>()

function Comp() {

  return <UT.Root {...typehelper("endpoint", { ... })}>...

@veloii
Copy link
Author

veloii commented Sep 14, 2024

How many component parts are there?

I'm working on Button, AllowedContent, ClearButton and Dropzone (4) at the moment. I'll try do another PR for InlineMedia and FileList (2). I'll implement it without tree-shaking for now and measure the bundle size to see if it's worth it.

@juliusmarminge
Copy link
Collaborator

I have a feeling each of those is not adding a lot on their own? It's the underlying core logic that eats most of the bytes and those need to be included regardless?

@veloii
Copy link
Author

veloii commented Sep 14, 2024

Yeah most of the components themselves are tiny. I've got a draft: #947.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:packages issue regarding one of the uploadthing packages ✨ enhancement suggestion or feature request to improve uploadthing PRs Accepted Anyone feel free to pick this up
Projects
None yet
Development

No branches or pull requests

4 participants