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
Bug 1909236: Fix for overlapping remove pin icon on long nav items #7606
Bug 1909236: Fix for overlapping remove pin icon on long nav items #7606
Conversation
@jeff-phillips-18: This pull request references Bugzilla bug 1909236, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest |
1 similar comment
/retest |
9adc6ac
to
6abea87
Compare
/retest |
aria-label={ | ||
_.isString(content) | ||
? isBookmarked | ||
? t('dropdown~Remove bookmark {{content}}', { content }) | ||
: t('dropdown~Add bookmark {{content}}', { content }) | ||
: null | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit: Removed this here because this commit is part of #7608. So let us make the change and discussion there. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how I added this commit... sorry
@jeff-phillips-18 Overlapping works fine and would receive a lgtm from me. But the other commit is already part of #7608. I liked this change as well but have a small request in the other PR. If you can drop this commit we can add some badges to this PR here. 😄 |
@jeff-phillips-18: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
6abea87
to
494c0e9
Compare
494c0e9
to
9354c7c
Compare
Sorry, not sure how I added that commit but it is gone now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine. Thanks for cleaning up the history. Both PRs lgtm.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeff-phillips-18, jerolimov, rohitkrai03 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 |
@jeff-phillips-18: All pull requests linked via external trackers have merged: Bugzilla bug 1909236 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes:
https://issues.redhat.com/browse/ODC-5205
Analysis / Root cause:
The text for the nav item is truncated when it hits the end of the nav bar area.
Solution Description:
On hover, truncate the text for the nav item when it hits the area where the remove icon is shown.
Screen shots / Gifs for design review:
Unit test coverage report:
Browser conformance:
/bug