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 TextWithIcon icon alignment #4202

Merged
merged 2 commits into from Oct 16, 2023

Conversation

jepunnerud
Copy link
Contributor

Description

Fixed wrapping of TextWithIcon component, thus also fixing the alignment of icons. Did this by wrapping the content (the text) in a span, and adding a div wrapping the span with "width: max-content".

Result

before
image

after
image

Testing

  • I have thoroughly tested my changes.

Tested locally on both laptop, desktop and mobile views.


@Arashfa0301 Arashfa0301 added the small-fix Pull requests that fix something small label Oct 16, 2023
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! It has been bugging me for ages! <3

Please look into my comment though.

content,
tooltipContentIcon,
iconRight = false,
size,
}: Props) => {
return (
<Flex alignItems="center" className={className}>
Copy link
Member

Choose a reason for hiding this comment

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

The className is used other places than on the event page, so the removal of this will break all of those. Any reason for removing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really. Added it back now.

@ivarnakken ivarnakken added review-needed Pull requests that need review changes-requested Pull requests with requested changes bug-fix Pull requests that fix a bug labels Oct 16, 2023
Copy link
Contributor

@Arashfa0301 Arashfa0301 left a comment

Choose a reason for hiding this comment

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

Looks good. Thank for the contribution ❤️

Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

Assuming you also checked the removal of the min-width for the other use cases of the component 😄

Thanks again!

@ivarnakken ivarnakken enabled auto-merge (squash) October 16, 2023 21:24
@ivarnakken ivarnakken added approved Pull requests that have been approved and removed changes-requested Pull requests with requested changes labels Oct 16, 2023
@ivarnakken ivarnakken merged commit 5abe90d into webkom:master Oct 16, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug review-needed Pull requests that need review small-fix Pull requests that fix something small
Projects
None yet
3 participants