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.

Sign up for GitHub

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

Jump to bottom

Migrate masthead to PF4 and clean-up PF3 CSS #1584

Merged
merged 10 commits into from
Jun 7, 2022
Merged

Migrate masthead to PF4 and clean-up PF3 CSS #1584

merged 10 commits into from
Jun 7, 2022

Conversation

rszwajko
Copy link
Member

@rszwajko rszwajko commented Apr 13, 2022 •
edited
Loading

Depends on PR #1549

Changes:

  1. use PF4 Page and Masthead ecosystem (includes Breadcrumb and Notifications)
  2. remove PF3 related styling and fix the resulting regressions

TODO:

  1. re-implemement functionalities that rely on react-console EDIT fixed in follow up PR Migrate to PF4 based react-console #1585

@rszwajko rszwajko requested review from sjd78 and sgratch April 13, 2022 12:21
@ovirt-infra
Copy link

All tests passed

@rszwajko
Copy link
Member Author

Masthead - full size

image

Masthead - minimal

image

Notifications - full size

image

Notifications - minimal

image

This was referenced Apr 13, 2022
Merged
Merged
@rszwajko rszwajko added the PF4 label Apr 13, 2022
@ovirt-infra
Copy link

All tests passed

@rszwajko rszwajko changed the title [WIP] Migrate masthead to PF4 and clean-up PF3 CSS Migrate masthead to PF4 and clean-up PF3 CSS Apr 29, 2022
@ovirt-infra
Copy link

All tests passed

@ovirt-infra
Copy link

All tests passed

@ovirt-infra
Copy link

All tests passed

@rszwajko rszwajko mentioned this pull request May 27, 2022
Merged
@ovirt-infra
Copy link

All tests passed

@ovirt-infra
Copy link

All tests passed

@sjd78
Copy link
Member

sjd78 commented Jun 3, 2022 •
edited
Loading

FWIW - The masthead height and top line etc was made to match that of webadmin. As-is right now, the mastheads are now different sizes.

Followup thought -- maybe update the webadmin masthead with the PF4 sizes instead?

@sjd78
Copy link
Member

sjd78 commented Jun 3, 2022

Logged out view looks good.

Note: To get to is without just waiting, use the Redux browser plugin and dispatch an action with the content:

{ type: 'LOGOUT', payload: { isManual: false } }

sjd78
sjd78 reviewed Jun 3, 2022
View reviewed changes
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

Masthead, notification drawer for events, user menu, about box, error boundary, logout screen, bad browser screen ... all looking good.

The only thing really to consider is if we still want the admin portal mast head to match the VM portal mast head. They matched before (size/height).

branding/brand.css Show resolved Hide resolved
src/components/OvirtApiCheckFailed.js Show resolved Hide resolved
src/components/VmUserMessages/index.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/intl/messages.js Show resolved Hide resolved
rszwajko added 2 commits June 6, 2022 10:06
@rszwajko
Migrate VM List screen to PF4
9d5f621 
Includes:
1. toolbars
2. tooltips
3. spinner
4. masthead icons

Fixes:
1. use translation independent filters
2. allow filter multiselection
@rszwajko
Include scrollbar width in VM List container padding
d94c13a
@ovirt-infra
Copy link

All tests passed

rszwajko added 3 commits June 6, 2022 18:42
@rszwajko
Re-enable sorting via toolbar icon
dfdd1f1
@rszwajko
Keep the xpath selector used by OST to check VM count
b206d6f 
Integration tests (OST) expect VM count under specific xpath:
'//div[@Class='col-sm-12']/h5'

Preserve this path:
1. keep the class 'col-sm-12' as marker class
2. keep the structure: <div> followed by <h5>
@rszwajko
Migrate Create VM Wizard to PF4
f7d5097 
Key changes:
1. reimplement Network and Storage steps using TableComposable
2. add 'final' step after the Summary for tracking progress
rszwajko added 5 commits June 6, 2022 19:09
@rszwajko
Add title to SummaryReview step
dea1f5b 
Bring back the title used previously in the PF3 version.

Includes styling changes in BasicSettings step:
1. re-enable validation for VM name
2. add more space above sysprep/cloud init section
@rszwajko
Migrate page layout to PF4 Page/Masthead
b1cb62d 
Includes:
1. Breadcrumb
2. NotificationDrawer
3. AboutDialog refactoring to functional component
@rszwajko
Remove PF3 styles
eae541e
@rszwajko
Migrate NoLogin and ErrorContent pages to PF4
ee9dc8f
@rszwajko
Extract IDs used by integration tests for logout
007eb00
@ovirt-infra
Copy link

All tests passed

@rszwajko
Copy link
Member Author

rszwajko commented Jun 7, 2022

/ost

sjd78
sjd78 approved these changes Jun 7, 2022
View reviewed changes
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

LGTM

@sjd78 sjd78 merged commit 703b8f7 into oVirt:master Jun 7, 2022
sjd78 added a commit to sjd78/ovirt-web-ui that referenced this pull request Jun 9, 2022
@sjd78
Expire translations after zanata push
7c3624e 
After merging PRs and pushing their string changes, pull the
translations.  This picks up translation invalidations (English text
has changed), helping to make sure the translations are not wrong.

PRs to consider:
  - oVirt#1533
  - oVirt#1537
  - oVirt#1539
  - oVirt#1540
  - oVirt#1543
  - oVirt#1549
  - oVirt#1564
  - oVirt#1584
  - oVirt#1585 ** pending merge
  - oVirt#1592 ** pending merge
@sjd78 sjd78 mentioned this pull request Jun 9, 2022
Merged
sjd78 added a commit to sjd78/ovirt-web-ui that referenced this pull request Jun 9, 2022
@sjd78
Expire translations after zanata push
af3a4c2 
After merging PRs and pushing their string changes, pull the
translations.  This picks up translation invalidations (English text
has changed), helping to make sure the translations are not wrong.

PRs to consider:
  - oVirt#1533
  - oVirt#1537
  - oVirt#1539
  - oVirt#1540
  - oVirt#1543
  - oVirt#1549
  - oVirt#1564
  - oVirt#1584
  - oVirt#1585 ** pending merge
  - oVirt#1592 ** pending merge
sjd78 added a commit to sjd78/ovirt-web-ui that referenced this pull request Jun 14, 2022
@sjd78
Expire translations after zanata push
eafeb05 
After merging PRs and pushing their string changes, pull the
translations.  This picks up translation invalidations (English text
has changed), helping to make sure the translations are not wrong.

PRs to consider:
  - oVirt#1533
  - oVirt#1537
  - oVirt#1539
  - oVirt#1540
  - oVirt#1543
  - oVirt#1549
  - oVirt#1564
  - oVirt#1584
  - oVirt#1585
  - oVirt#1592 ** pending merge
sjd78 added a commit to sjd78/ovirt-web-ui that referenced this pull request Jun 14, 2022
@sjd78
Expire translations after zanata push
aabcf3f 
After merging PRs and pushing their string changes, pull the
translations.  This picks up translation invalidations (English text
has changed), helping to make sure the translations are not wrong.

PRs to consider:
  - oVirt#1533
  - oVirt#1537
  - oVirt#1539
  - oVirt#1540
  - oVirt#1543
  - oVirt#1549
  - oVirt#1564
  - oVirt#1584
  - oVirt#1585
  - oVirt#1592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjd78 sjd78 sjd78 approved these changes

@sgratch sgratch Awaiting requested review from sgratch

Assignees
No one assigned
Labels
PF4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rszwajko @ovirt-infra @sjd78