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

Create TextWithIcon component #3590

Merged
merged 3 commits into from Feb 22, 2023
Merged

Conversation

ingraso
Copy link
Member

@ingraso ingraso commented Feb 18, 2023

Description

Text is displayed with icons throughout the whole webapp. However, there is no component or common way of doing this. With the <TextWithIcon /> component there is now a generalized component that can be used for this purpose. An example of usage can be seen here:
image

There should be no visual changes to the text with icons.

Also fixes some TS errors and moves the tooltip on <EventDetail /> to be on the icon rather than the text.

Result

Before
image

After
image

Testing

  • I have thoroughly tested my changes.

Tested by clicking through where changes have been done.


Resolves ABA-293

@ingraso ingraso added enhancement Pull requests that make enhancements, instead of just purely new features review-needed Pull requests that need review technical-debt Pull requests that reduces technical debt labels Feb 18, 2023
@ingraso ingraso self-assigned this Feb 18, 2023
@linear
Copy link

linear bot commented Feb 18, 2023

ABA-293 Create TextWithIcon component

At the moment, we have the same layout of having an icon next to some textual content. Therefore, this should be generalized into a separate component that can be used throughout the webapp.

An example of usage:

image.png

@ingraso ingraso requested a review from a team February 18, 2023 20:17
@ivarnakken ivarnakken added the changes-requested Pull requests with requested changes label Feb 19, 2023
Copy link
Member

@LudvigHz LudvigHz left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Nice to standardize stuff like this 💯 Just fix up in Olav's comment and we should be good here

To generalize the usage of text together with icons. At the moment we
have a lot of different ways of doing this.
@ingraso ingraso force-pushed the aba-293-create-textwithicon-component branch from 01cae5a to 76e6d63 Compare February 21, 2023 14:50
Previously it could only be ReactElement, but it should also accept a
simple string for simplicity reasons.
@ingraso ingraso force-pushed the aba-293-create-textwithicon-component branch from 76e6d63 to 8e273b7 Compare February 21, 2023 15:04
Copy link
Contributor

@ollfkaih ollfkaih 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 now!

@ivarnakken ivarnakken added approved Pull requests that have been approved and removed changes-requested Pull requests with requested changes review-needed Pull requests that need review labels Feb 22, 2023
@ingraso ingraso merged commit ac747ac into master Feb 22, 2023
@ingraso ingraso deleted the aba-293-create-textwithicon-component branch February 22, 2023 14:24
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 enhancement Pull requests that make enhancements, instead of just purely new features technical-debt Pull requests that reduces technical debt
Projects
None yet
4 participants