fuma-nama / fumadocs

The beautiful docs framework with Next.js. Alternative to Nextra
https://fumadocs.vercel.app
MIT License
1.92k stars 116 forks source link

fix: make <Accordions /> 'type' prop optional #1080

Closed nktnet1 closed 1 week ago

nktnet1 commented 1 week ago

The documentation for accordian has the following as the example usage:

<Accordions>
  <Accordion title="My Title">My Content</Accordion>
</Accordions>

However, this resulted in the error:

image
Raw Error ``` Type '{ children: Element; }' is not assignable to type 'IntrinsicAttributes & ((Omit | Omit) & RefAttributes<...>)'. Property 'type' is missing in type '{ children: Element; }' but required in type 'Omit'.ts(2322) index.d.mts(23, 5): 'type' is declared here. (alias) const Accordions: ForwardRefExoticComponent<(Omit | Omit) & RefAttributes<...>> import Accordions ```

This PR makes the accordion type optional. I've defined CustomProps for now, and plan to add the updateAnchor option similar to the component

changeset-bot[bot] commented 1 week ago

🦋 Changeset detected

Latest commit: 27ed96a750a4f0c8d09822819d1dea7fd75ce19b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ------------------- | ----- | | fumadocs-ui | Patch | | fumadocs-openapi | Patch | | fumadocs-core | Patch | | create-fumadocs-app | Patch |

Not sure what this means? Click here to learn what changesets are.

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

vercel[bot] commented 1 week ago

@nktnet1 is attempting to deploy a commit to the Fuma Team on Vercel.

A member of the Team first needs to authorize it.

fuma-nama commented 1 week ago

It is expected, the usage in TSX files requires type property. It’s only added as a fallback for convenience in MDX files

fuma-nama commented 1 week ago

Also the accordion component is mainly for displaying FAQs and additional sections, I prefer to have a copy button instead of changing hash when opening it, it’s more intuitive for people to quickly link to a section or answer

nktnet1 commented 1 week ago

It is expected, the usage in TSX files requires type property. It’s only added as a fallback for convenience in MDX files

ah alright, understood. Should this be documented somewhere? Feel free to close this PR.

Also the accordion component is mainly for displaying FAQs and additional sections, I prefer to have a copy button instead of changing hash when opening it, it’s more intuitive for people to quickly link to a section or answer

That makes sense - I just thought it'll be good to support both. On our side, we have people (not many) getting confused by the hash not "working" when they selected a different accordion (e.g. they were initially sent a working link, tried to open a different accordion, then send the URL to someone else). For context, we use the accordion to show a weekly schedule of events.

I could also implement this client side though, so not an issue. Won't make a PR for it in that case. Thanks @fuma-nama.

fuma-nama commented 1 week ago

Yeah it’s better to implement it on user land. I will improve the docs later