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
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.
IMO, the version badge should come before the platform ones.
We can also go the Android way and have it always to the right
We may also want to default to showing "3.0" if no version is provided to be consistent. So there's no ambiguity about which version something is available from.
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', |
There was a problem hiding this comment.
| flexDirection: 'row', | |
| flexDirection: 'row', | |
| flexWrap: 'wrap', |
I think it looks nicer this way on narrow screens.
vs
There was a problem hiding this comment.
I agree. I also moved it to the left on smaller screens:
j-piasecki
left a comment
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?
Sure, that's my plan
But I guess your point is to reduce it. To be honest, I'd leave that as it is now. |
2 participants
Description
This PR, similarly to #3975, adds badges for versions for the features.
Darkmode
Lightmode
Test plan
Read docs 🤓