Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…or.mdx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
j-piasecki
left a comment
There was a problem hiding this comment.
I'm not sold on the styling. I don't think the version badge should be that attention-grabbing.
| <PlatformBadge key={platform} platform={platform} /> | ||
| ))} | ||
|
|
||
| {version && <VersionBadge version={version} />} |
There was a problem hiding this comment.
There was a problem hiding this comment.
I'm not sold on the styling. I don't think the version badge should be that attention-grabbing.
I agree. Android convention looks good, but do we want it as a plain text, or have it in a badge like platform, with with different color (like white badge with black text)?
There was a problem hiding this comment.
Styles updated in 36b7fe0. I've also removed unused import from FlowChart (been there to steal useColorMode)
There was a problem hiding this comment.
So if styling looks good, the question is where do we want to put them. Props like disableReanimated and so on? Or do we wait for something new.
There was a problem hiding this comment.
So if styling looks good, the question is where do we want to put them. Props like disableReanimated and so on? Or do we wait for something new.
That's a good question - on one hand, I like the way Android does it, so everything is clearly labeled, but on the other, that's a lot of boxes. It could be ok if we reduce it only to the version instead of Available from ..., but it's not as clean then.
TL;DR: I don't know
| }, | ||
|
|
||
| badge: { | ||
| borderRadius: 10, |
There was a problem hiding this comment.
| borderRadius: 10, | |
| borderRadius: 16, |
IMO it looks nicer.
| const styles = StyleSheet.create({ | ||
| container: { | ||
| display: 'flex', | ||
| flexDirection: 'row', |
j-piasecki
left a comment
There was a problem hiding this comment.
- I forgot about it yesterday, but when I was checking it, I was experimenting with reducing the font size for the version badge (I think 12px). I'm fine with both, so I'll leave it up to you.
- Can you please merge this after the platform badges?
There was a problem hiding this comment.
Pull request overview
This PR updates the docs UI to support showing multiple “platform” badges alongside an (optional) “available from version” badge, and migrates existing docs pages to use the new header component.
Changes:
- Remove an unused React hook import in the flowchart example.
- Add a new
HeaderWithBadgescomponent + CSS module styling (platform badges + optional version badge). - Update multiple MDX docs pages to use
HeaderWithBadgesinstead ofHeaderWithBadge.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/docs-gesture-handler/src/examples/CallbacksFlowCharts/FlowChart.jsx | Removes unused useLayoutEffect import. |
| packages/docs-gesture-handler/src/components/HeaderWithBadges/index.tsx | Introduces new header component rendering platform badges and an optional version badge. |
| packages/docs-gesture-handler/src/components/HeaderWithBadges/styles.module.css | Adds styling for platform badges and theme-aware version badge. |
| packages/docs-gesture-handler/docs/gestures/use-tap-gesture.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/gestures/use-pan-gesture.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/gestures/use-native-gesture.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/gestures/use-long-press-gesture.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/gestures/use-hover-gesture.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/gestures/use-fling-gesture.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/gestures/_shared/base-gesture-config.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/fundamentals/gesture-detector.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/components/reanimated_swipeable.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/components/reanimated-drawer-layout.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/components/pressable.mdx | Migrates to HeaderWithBadges. |
| packages/docs-gesture-handler/docs/components/buttons.mdx | Migrates to HeaderWithBadges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {platforms | ||
| ?.map((platform) => platform.toLowerCase() as Platform) | ||
| .sort() |
There was a problem hiding this comment.
platforms is a required prop in HeaderWithBadgesProps, but the render uses optional chaining (platforms?.map(...)). This makes the type contract misleading and can hide mistakes at call sites. Either remove the optional chaining (and keep platforms required) or make the prop optional and define an explicit empty-state behavior.





Description
This PR, similarly to #3975, adds badges for versions for the features.
Darkmode
Lightmode
Test plan
Read docs 🤓