Skip to content

Margin and fontSize alignment of TagCount #225

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

Merged
merged 1 commit into from
Aug 1, 2024

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

@@ -12,7 +12,8 @@ const useStyles = createUseStyles({
},

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

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')

@@ -12,7 +12,8 @@ const useStyles = createUseStyles({
},

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

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
Collaborator

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
Collaborator

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
6 checks passed
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