Dialog and Modal fix for web #2450
Conversation
| @@ -290,6 +291,11 @@ class Button extends PureComponent<Props, ButtonState> { | |||
| if (typeof iconSource === 'function') { | |||
| return iconSource(iconStyle); | |||
| } else { | |||
| if (Constants.isWeb) { | |||
There was a problem hiding this comment.
Any reason we're not using Icon all of the time? both for web and mobile?
There was a problem hiding this comment.
Yes, if we will user Icon and the user don't pass any tintColor the Icon will get the default color.
So if we change it to Icon it will break some users.
There was a problem hiding this comment.
Ok, we need to migrate to Icon IMO, we can do it v8.
Can you add it to the v8 spreadsheet
There was a problem hiding this comment.
Done added to the spreadsheet.
src/components/icon/index.tsx
Outdated
| if (typeof source === 'string' && isBase64ImageContent(source) && Constants.isWeb) { | ||
| return ( | ||
| <Image | ||
| {...others} | ||
| ref={ref} | ||
| source={iconSource} | ||
| style={[ | ||
| style, | ||
| margins, | ||
| iconSize, | ||
| shouldFlipRTL && styles.rtlFlipped, | ||
| !!tintColor && { | ||
| tintColor | ||
| } | ||
| ]} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
It looks like this render is exactly the same as the default render at the bottom..
There was a problem hiding this comment.
I'll move it into renderImage function.
Should I move the Svg into renderSvg function also?
There was a problem hiding this comment.
Sure, it can make a little more organized 👌
src/components/icon/index.tsx
Outdated
| const iconSize = size ? {width: size, height: size} : undefined; | ||
| const webIconSize = Constants.isWeb | ||
| ? {width: size || defaultWebIconSize, height: size || defaultWebIconSize} | ||
| : undefined; | ||
| const iconSize = size ? {width: size, height: size} : webIconSize; |
There was a problem hiding this comment.
I suggest encapsulating this logic under a hook for useIconSize. It will handle all the logic and return the final size.
Right now it's getting a little unreadable
There was a problem hiding this comment.
I'm not sure if we need to have a hook for this case.
I did refactor to this logic:
const iconSize = Constants.isWeb ? size || defaultWebIconSize : size; return {width: iconSize, height: iconSize};
Since the width and the height get the same size the only thing that matters is the web environment.
If the user is using the web platform let's try to get the size else will get the defaultWebIconSize.
There was a problem hiding this comment.
See if you can do something like that when you destruct the size prop from props with a default prop
const {
size = Constants.isWeb ? defaultWebIconSize : undefined,
tintColor,
style,
.....
} = props;
… changed the logic back
Description
The picker component supports the web, fixed issues: useDialog mode, Multi-select mode, and TopBar icons fixed.
Changelog
The picker component supports the web, fixed issues: useDialog mode, Multi-select mode, and TopBar icons fixed.