Add values for all border corners #10
Conversation
src/FocusRingContext.tsx
Outdated
|
|
||
| if (!topLeft || !topRight || !bottomRight || !bottomLeft) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
hello! I am not sure if it's okay for anyone to comment on PR's? if it's not, I'm sorry 😔
just an ideia: maybe it would be nice to support elements that do not have all the corners rounded.
example: border-radius: 10px 10px 0 0 (only top borders are rounded).
I believe this would be the default behaviour for browsers that can get the full border-radius string.
and, if there are no borders at all, we return undefined. what do you think?
There was a problem hiding this comment.
It's absolutely welcomed for anyone to comment! Thanks for point this out, too.
I agree that we should allow any subset of sides to have a border radius. I can specifically think of examples like the start and ends of "button groups" that might want smaller/larger radiuses on the outer sides than the inners, or things like partially-rounded shapes (a square with one rounded corner, for example).
This code can just default to 0 for corners that don't have a defined radius value, and that should cover it.
There was a problem hiding this comment.
Thank you for the comments! I was thinking about this use case as well, but wasn't sure if we should default it to 0. Will fix it!
src/FocusRingContext.tsx
Outdated
|
|
||
| if (!topLeft || !topRight || !bottomRight || !bottomLeft) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
It's absolutely welcomed for anyone to comment! Thanks for point this out, too.
I agree that we should allow any subset of sides to have a border radius. I can specifically think of examples like the start and ends of "button groups" that might want smaller/larger radiuses on the outer sides than the inners, or things like partially-rounded shapes (a square with one rounded corner, for example).
This code can just default to 0 for corners that don't have a defined radius value, and that should cover it.
src/FocusRingContext.tsx
Outdated
| @@ -24,6 +24,10 @@ function setActiveRingContextManager(manager: FocusRingContextManager) { | |||
| } | |||
| } | |||
|
|
|||
| function parseBorderRadius(radius: string) { | |||
There was a problem hiding this comment.
Since this function is called with an optional property chain (styles[0]?.property), this could actually be an undefined value, so radius: string | undefined. I'd leave it in the long-hand rather than radius?: string since the argument is still required, it may just be undefined.
There was a problem hiding this comment.
Good point! Fixed it.
Fixes #7.
Screenshot:
