Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ListItemLink alignment #1088

Merged
merged 7 commits into from
Mar 28, 2023
Merged

Conversation

chaitanyadeorukhkar
Copy link
Collaborator

@chaitanyadeorukhkar chaitanyadeorukhkar commented Mar 23, 2023

React native has a vertical alignment bug that causes nested text which also has its own view inside to cause misalignment. This PR wraps all the children of list item (text and view) in a flex and aligns them.

Ref: facebook/react-native#31955

Before this PR:
Simulator Screen Shot - iPhone 14 Pro - 2023-03-28 at 11 07 59

After this PR:
Simulator Screen Shot - iPhone 14 Pro - 2023-03-28 at 11 09 24

@changeset-bot
Copy link

changeset-bot bot commented Mar 23, 2023

🦋 Changeset detected

Latest commit: 1f81f68

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@razorpay/blade Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 23, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1f81f68:

Sandbox Source
razorpay/blade: basic Configuration

@@ -186,7 +186,7 @@ const getProps = ({
isVisited,
}) as IconProps['color'],
fontSize: size === 'medium' ? 100 : 75,
lineHeight: size === 'medium' ? 'm' : 's',
lineHeight: size === 'medium' ? 'l' : 's',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an incorrect implementation 🙈. It is l instead of m on Figma as well.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023

✅ PR title follows Conventional Commits specification.

children,
size,
}: {
children: (React.ReactChild | React.ReactFragment | React.ReactPortal)[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we kept this so specific? eg: Fragment, ReactPortal.

Can we use React.ReactNode instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had done it to suppress a warning. Made it ReactNode now. Draft PR hai, have mercy 😢

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol sorry, was reviewing this at airport :p

@kamleshchandnani kamleshchandnani merged commit c7c6605 into master Mar 28, 2023
@kamleshchandnani kamleshchandnani deleted the fix/list-item-link-alignment branch March 28, 2023 06:47
anuraghazra pushed a commit that referenced this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants