Skip to content

Conversation

@jiridostal
Copy link
Contributor

RHINENG-7746

The tag number and icon seems to be misaligned and font-size is off in the context of surrounding content.

Before:
fontSize: 16px
margin: 10px
image

After:
fontSize: 14px
margin: 6px
image

Any changes or improvements to this PR are welcome.

@patternfly-build
Copy link

patternfly-build commented Jul 30, 2024

tagText: {
marginLeft: 10
marginLeft: 6,
fontSize: 14
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fontSize: 14
fontSize: 'var(--pf-v5-global--FontSize--sm')


tagText: {
marginLeft: 10
marginLeft: 6,
Copy link
Contributor

Choose a reason for hiding this comment

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

this one could be replaced with a variable as well
the closest is probably "--pf-v5-global--spacer--sm" 0.5rem.
@kaylachumley according to PF rules, do you think we should go with hardcoded 6px or that variable? Thank you

@fhlavac
Copy link
Contributor

fhlavac commented Jul 31, 2024

@jiridostal looks good, thank you. Let me just check quickly on the variables. It's preferred to use them instead of hardcoded values

@kaylachumley
Copy link

Let's use the variable over the hard coded values so all changes can be adopted with future variable updates! Thanks!

@kaylachumley
Copy link

Also, The tag count styling seems to be a little off, not sure if this is still in the works with another issue on text and icon color

@fhlavac
Copy link
Contributor

fhlavac commented Jul 31, 2024

@kaylachumley yeah, the remaining issues will be tracked separately. Thank you!

Copy link

@vkrizan vkrizan left a comment

Choose a reason for hiding this comment

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

❤️

@fhlavac fhlavac merged commit 760b59b into patternfly:main Aug 1, 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.

5 participants