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

Prevent RedHat supported icon on application card title from being alone on a line #947

Merged

Conversation

Gkrumbach07
Copy link
Member

@Gkrumbach07 Gkrumbach07 commented Feb 10, 2023

Description

Fix: #879
Split application card title string so that the last word is always on the same line as the RedHat supported icon.

How Has This Been Tested?

  1. Hard coded sample titles to test edge cases
    • Short title (title less than card width)
      • icon should be inline with title with no overflow
    • Long title (title 1.5 times the card width)
      • icon should be inline with the overflowed text on the second line
    • Goldilocks title (title exactly the width of the card)
      • this previously caused the icon to overflow to the next line alone
      • now the last word in the title and the icon should overflow to the next line
  2. Compare with previous build to verify title and icon alignment is the same

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@Gkrumbach07 Gkrumbach07 changed the title prevent supported icon from being alone on a line Prevent RedHat supported icon on application card title from being alone on a line Feb 10, 2023
frontend/src/components/SupportedAppTitle.tsx Outdated Show resolved Hide resolved
frontend/src/components/SupportedAppTitle.tsx Outdated Show resolved Hide resolved
frontend/src/components/SupportedAppTitle.tsx Outdated Show resolved Hide resolved
frontend/src/components/SupportedAppTitle.tsx Outdated Show resolved Hide resolved
frontend/src/components/SupportedAppTitle.tsx Outdated Show resolved Hide resolved
@Gkrumbach07 Gkrumbach07 dismissed andrewballantyne’s stale review February 13, 2023 18:07

fixed issue in a new commit

@openshift-ci openshift-ci bot added the lgtm label Feb 14, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 14, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@andrewballantyne
Copy link
Member

FWIW -- I am not a huge fan of the padding override -- I have followed up with PF here: https://patternfly.slack.com/archives/C4FM977N0/p1676392165096129

@openshift-merge-robot openshift-merge-robot merged commit a3a421d into opendatahub-io:main Feb 14, 2023
strangiato pushed a commit to strangiato/odh-dashboard that referenced this pull request Oct 18, 2023
…one on a line (opendatahub-io#947)

* prevent supported icon from being alone on a line

* Update frontend/src/components/SupportedAppTitle.tsx

Co-authored-by: Andrew Ballantyne <8126518+andrewballantyne@users.noreply.github.com>

* removed function, added aria labels

* removed aria labels, made icon accessible

---------

Co-authored-by: Andrew Ballantyne <8126518+andrewballantyne@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Explore Card Title & Icon - Minor UX Improvement
3 participants