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

Fixes #33876 - Fix content view card styles #9802

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Nov 19, 2021

What are the changes introduced in this pull request?

  • Change CV card layout to a single column so that longer CV names have more room and look okay
  • Bump font size down to 14px for CV name and version link
  • Increase card width from 3 to 4 for lg-sized screens (units are twelfths of the overall grid, so the card now takes up a third of the width on lg-size screens)
  • Fix capitalization

cv_card
See the Redmine issue #33876 for the before pic

Considerations taken when implementing this change?

I removed the Patternfly <DescriptionList> components because they weren't adding any benefits. They don't even create <dt>, <dl> or <dd> elements; they were just spans with classes.

I put some flex containers in to guarantee that

  • Content view icon and "Content view" text are always on the same line
  • The above and the actual CV name are always on a different line
  • The "Version in use" label and the actual version name are always on a different line

however, the line with the CV name and the environment can still wrap if it needs to.

What are the testing steps for this pull request?

  • Look at the CV card for a host in Default Organization View
  • Test with several different screen widths
  • Reload the page after changing screen width (unfortunately, some of our styles result from JS and not CSS)

@theforeman-bot
Copy link

Issues: #33876

@jturel
Copy link
Member

jturel commented Nov 30, 2021

Looks like a good set of improvements. The only comment I have is that the space between the cv icon and 'Content view' seems unnecessarily large. Too much of a right margin on the icon perhaps?

@jeremylenz
Copy link
Member Author

@jturel thanks, should be fixed now!

Also added alignItems: 'center' so that the text and icon look better together.

Copy link
Member

@jturel jturel left a comment

Choose a reason for hiding this comment

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

ACK!

@jeremylenz jeremylenz merged commit de3dfa7 into Katello:master Dec 1, 2021
@jeremylenz jeremylenz deleted the 33876-cv-card-fix branch December 1, 2021 20:58
chris1984 pushed a commit that referenced this pull request Dec 7, 2021
* Fixes #33876 - Fix content view card styles
* Refs #33876 - fix icon spacing & alignment

(cherry picked from commit de3dfa7)
chris1984 pushed a commit that referenced this pull request Dec 7, 2021
* Fixes #33876 - Fix content view card styles
* Refs #33876 - fix icon spacing & alignment

(cherry picked from commit de3dfa7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants