search

nandorojo / zeego

Menus for React (Native) done right.
https://zeego.dev
MIT License
1.42k stars 41 forks source link

fix: enable custom styling of arrow components #68

Closed nderscore closed 7 months ago

nderscore commented 7 months ago

Fixes #67

This PR updates the Arrow component of both menu types to more closely map to the props supported by Radix's Arrow.

By default, Radix's Arrow renders a custom <svg> arrow element and accepts all props that an <svg> does. The expected means of styling it are either to pass in a fill attribute, a className, a style object with fill, or to use its asChild prop and render a custom component.

Currently, Zeego's Arrow renders a custom <View> inside Radix's arrow, which gets the style prop passed to it. This child is never rendered by Radix, so these styles are never used.

To remedy this, I've updated the Arrow components to accept all of the attributes mentioned above (fill, className, style)

I also changed the way props are passed to the Arrow to use object spreading - this way, if someone finds another reason to pass additional HTML attributes to the arrow <svg> there's an easy escape hatch already in place - they just need to bypass Typescript.

There is also one one small "breaking" change:

Because the <svg> isn't being rendered by a <View> and because it needs to be able to support the fill property in order to be useful, the style prop on the Arrow is no longer a ViewStyle, but a normal CSSProperties object. I stuck a warning in the docs calling this out. If anyone was passing StyleSheet styles there before, those styles weren't being rendered anyway :sweat_smile:

While I was in there updating docs, I also updated the Trigger prop tables to include the asChild prop, which I noticed was missing. Happy to remove that change it if was intentionally undocumented.

vercel[bot] commented 7 months ago

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

Name Status Preview Comments Updated (UTC)
zeego-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 16, 2023 3:00am
nderscore commented 7 months ago

Side-note: I wasn't able to run this through the linter because of a missing config:

Oops! Something went wrong! :(

ESLint: 7.32.0

ESLint couldn't find the config "nando" to extend from. Please check that the name of the config is correct.

The config "nando" was referenced from the config file in "/...../zeego/.eslintrc".

But I did run all the updated files through prettier.

nandorojo commented 7 months ago

Hm I'll ignore the ESLint that's fine. Merged.