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

chore: drop patternfly #4762

Merged
merged 1 commit into from
Nov 14, 2023
Merged

chore: drop patternfly #4762

merged 1 commit into from
Nov 14, 2023

Conversation

benoitf
Copy link
Collaborator

@benoitf benoitf commented Nov 10, 2023

What does this PR do?

All usage of patternfly have been dropped so it's time to be able to remove patternfly

I also remove the overrides that were done on top of patternfly as we no longer them

I changed default colour of text to be white and bring font-awesome font/css before it was being provided through patternfly but as it is removed it was not there anymore

we might have some little glitches this is why I wanted to get this merge long time before a new release so we have to identify them/fix them

We've lost the Redhat Font also, I don't know if we want to pickup another default font.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes #1617
fixes #4319

How to test this PR?

Look at the UI

also remove the overrides that were done on top of patternfly

change default color of text to be white and bring font-awesome font/css
before it was being provided through patternfly but as it is removed it was not there anymore

fixes containers#1617
fixes containers#4319

Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf benoitf requested a review from a team as a code owner November 10, 2023 12:50
@benoitf benoitf requested review from jeffmaury, cdrage and deboer-tim and removed request for a team November 10, 2023 12:50
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM although I see some light differences in font on some screens (eg Create container dialog):

After:
image

1.5.3:
image

@benoitf
Copy link
Collaborator Author

benoitf commented Nov 13, 2023

@jeffmaury
so how do we want to proceed there ?

Try to fix all issues coming from dropping patternfly before the merge or merge early and do follow-up PRs to get all breakages being fixed.

Also, do you prefer with the new font or with older font size

@jeffmaury
Copy link
Contributor

@jeffmaury so how do we want to proceed there ?

Try to fix all issues coming from dropping patternfly before the merge or merge early and do follow-up PRs to get all breakages being fixed.

Also, do you prefer with the new font or with older font size

No for me it's not a blocker just recording for later issues

Copy link
Collaborator

@deboer-tim deboer-tim left a comment

Choose a reason for hiding this comment

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

The CSS override was setting a line height of 0.9rem, we might want to keep that for now just for less change and smaller screens.

I've been running with most of these changes or variants of them already and LGTM. I think we're close enough that it's better to switch early - more time for everyone to catch regressions, no risk of new dependencies on PF, benefits like single version of FA, etc.

@benoitf
Copy link
Collaborator Author

benoitf commented Nov 14, 2023

ok let's merge it now then

@benoitf benoitf merged commit 6bee813 into containers:main Nov 14, 2023
13 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.6.0 milestone Nov 14, 2023
@afbjorklund
Copy link
Contributor

Here is what I got, on Linux:

Before: before

After: after

The text is now invisible (until selected)

@benoitf
Copy link
Collaborator Author

benoitf commented Nov 14, 2023

thanks @afbjorklund

benoitf added a commit to benoitf/desktop that referenced this pull request Nov 14, 2023
fixes containers#4762 (comment)
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
benoitf added a commit that referenced this pull request Nov 14, 2023
fixes #4762 (comment)
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
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.

PatternFly cleanup UI: Drop patternfly and use only tailwind (technical debt)
5 participants