Issue 567 - Add border-radius support#568
Conversation
- apply table borders only to container - add basic test
| ref='table' | ||
| className={innerClasses.join(' ')} | ||
| style={tableStyle} | ||
| style={R.omit(BORDER_PROPERTIES_AND_FRAGMENTS, tableStyle)} |
There was a problem hiding this comment.
Omit the border stuff on the inner container and only apply to the outer container - otherwise the border gets doubled.
| ['borderLeftStyle', 'borderLeftStyle'], | ||
| ['borderLeftWidth', 'borderLeftWidth'], | ||
| ['borderRight', 'borderRight'], | ||
| ['borderRadius', 'borderRadius'], |
There was a problem hiding this comment.
Adding support for border-radius and the Python and React flavors
There was a problem hiding this comment.
whew, exciting file... any reason this couldn't be rewritten DRYer? Like:
const attrParts = [
// each attr only once
['align', 'content'],
['align', 'items'],
['alignment', 'adjust'],
...
];
const makeMap = attrParts => {
// generate the tripled list in all formats
}
export default new Map(makeMap(attrParts));Wouldn't need to be done now, but seems more maintainable and efficient.
There was a problem hiding this comment.
Actually this file is generated - forgot to push the generator update.
Goal of the generator is to prevent having to eval all of this at runtime and only have a table mapping for python/css/react casing. It's not dry because it's essentially a lookup table. That said, your comment below (#568 (comment)) is correct. Should generate a resolved array of these values instead of doing it on the fly for the filtered usages.
| borderRadius: '15px', | ||
| overflow: 'hidden' | ||
| } | ||
| }} />)); No newline at end of file |
There was a problem hiding this comment.
Basic test to make sure the radius is applied
| ) | ||
| ); | ||
|
|
||
| export const HEIGHT_AND_WIDTH_PROPERTIES: string[] = R.uniq( |
There was a problem hiding this comment.
heh could get rid of these R.uniq calls too if py2jsCssProperties was rewritten 🌴 and exposed the unique list too 😉
- update generator accordingly
|
Updated the generator based on @alexcjohnson comments. Using fragments, and without the regex involved this is indeed smaller than it was before and the initial gen cost in the FE is negligible. |
|
|
||
| export default new Map<string, string>(entries); | ||
|
|
||
| export const KnownCssProperties = camels; No newline at end of file |
| ref='table' | ||
| className={innerClasses.join(' ')} | ||
| style={tableStyle} | ||
| style={INNER_STYLE} |
There was a problem hiding this comment.
Ah that's way better 🏆
3 participants
Fixes #567