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
Migrate icons to PF4 components and upgrade Font Awesome #2076
Migrate icons to PF4 components and upgrade Font Awesome #2076
Conversation
ae1e943
to
6b61f07
Compare
6b61f07
to
6ca8423
Compare
6ca8423
to
5b16791
Compare
5b16791
to
6eec9d0
Compare
Sorry @spadgett, this one is big. But is relatively easy to review. |
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.
/lgtm
|
||
export const Phase: React.SFC<PhaseProps> = ({status}) => <span className={status === 'Failed' ? 'co-error' : ''}> | ||
{ status === 'Failed' && <i className="fa fa-ban" /> } {status} | ||
{ status === 'Failed' && <BanIcon data-test-id="ban-icon" /> } {status} |
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.
Nice solution to the test problem 👍
/test e2e-aws-console-olm |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
2 similar comments
/retest |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
6eec9d0
to
678ad50
Compare
Rebased. @spadgett, please tag. |
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.
This was approved before feature freeze and only needed a rebase.
/lgtm
92ebe16
to
1e6e3d7
Compare
Rebased due to conflict in |
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.
/lgtm
1e6e3d7
to
4b5b59c
Compare
Rebased due to conflict in resource-sidebar.jsx. @spadgett, please re-tag. |
infrastructure error /retest |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto, spadgett 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
Supports https://jira.coreos.com/browse/CONSOLE-1606
Note we still need to carry Font Awesome as a dependency for the cases where we use an icon as content in a pseudo element.