GridListItem - horizontal alignment support #3001
Conversation
| right = 'right' | ||
| } | ||
|
|
||
| export type ItemHorizontalAlignmentProp = ItemHorizontalAlignment | `${ItemHorizontalAlignment}`; |
There was a problem hiding this comment.
This is not a good name...
There was a problem hiding this comment.
Also, I don't see why we need to declare this type
| /** | ||
| * Content horizontal alignment (default is center) | ||
| */ | ||
| horizontalAlignment?: ItemHorizontalAlignmentProp; |
There was a problem hiding this comment.
Why do we need this prop? Why the user can't just pass 'alignItems' in the containerStyle prop, and 'textAlign' in the style prop?
|
@Inbal-Tish
|
Inbal-Tish
changed the title
| @@ -101,7 +112,7 @@ export interface GridListItemProps { | |||
| * Test ID for component | |||
| */ | |||
| testID?: string; | |||
| children?: React.ReactNode; | |||
| children?: React.ReactElement; | |||
src/components/image/index.tsx
Outdated
| @@ -73,7 +73,7 @@ export type ImageProps = Omit<RNImageProps, 'source'> & | |||
| /** | |||
| * Render an overlay with custom content | |||
| */ | |||
| customOverlayContent?: JSX.Element; | |||
| customOverlayContent?: React.ReactElement; | |||
There was a problem hiding this comment.
Why the children type change is relevant to this PR?
| Typography[typography], | ||
| color && {color}, | ||
| alignToStart && styles.contentAlignedToStart, | ||
| horizontalAlignmentStyle?.contentStyle |
There was a problem hiding this comment.
{textAlign: HorizontalAlignment} - can replace this object, just place it before this line:
alignToStart && styles.contentAlignedToStart, to avoid overriding it
|
@adids1221 The code is very messy and unclear IMO.
|
| {hasOverlay && <View style={[styles.overlay, this.getItemSizeObj()]}>{renderOverlay?.()}</View>} | ||
| <TextContainer {...textContainerStyle}> | ||
| {renderOverlay && <View style={[styles.overlay, itemSize]}>{renderOverlay()}</View>} | ||
| <View {...textContainerStyle}> |
There was a problem hiding this comment.
You changed the TextContainer from View or Fragment to View. This can result in alignment issues (if you remember the Icon bug...). Why did you make this change?
| style={[styles.container, alignToStart && styles.containerAlignedToStart, {width}, containerStyle]} | ||
| style={[ | ||
| styles.container, | ||
| {alignItems: alignToStart ? 'flex-start' : this.getContainerHorizontalAlignment(horizontalAlignment)}, |
There was a problem hiding this comment.
I would move the function call outside of the style
| @@ -73,7 +73,7 @@ export type ImageProps = Omit<RNImageProps, 'source'> & | |||
| /** | |||
| * Render an overlay with custom content | |||
| */ | |||
| customOverlayContent?: JSX.Element; | |||
| customOverlayContent?: React.ReactElement | React.ReactElement[]; | |||
There was a problem hiding this comment.
What these changes have to do with this PR?
adids1221
added
the
Important for Next Release
Description
Grid List Item support in horizontal alignment.
New
horizontalAlignmentprop to control the horizontal alignment of grid list item in all aspects - text, custom element, image and etc.Changelog
Grid List Item support in horizontal alignment
Additional info
ticket 4082