-
Notifications
You must be signed in to change notification settings - Fork 1k
Fixes #39059 - Update PageLayout to use PF5 #10857
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
base: develop
Are you sure you want to change the base?
Conversation
Lukshio
left a comment
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.
Hi, overall, it looks great. I didn't find any issues. I think we should set the searchBar width to 100% by default and use the toolbarItem widths to set the limitation.
webpack/assets/javascripts/react_app/components/SearchBar/SearchBar.scss
Show resolved
Hide resolved
webpack/assets/javascripts/react_app/routes/common/PageLayout/PageLayout.js
Outdated
Show resolved
Hide resolved
c9a6926 to
984e6ea
Compare
Lukshio
left a comment
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.
Css file can be replaced by adding alignSelf prop to ToolbarItem
webpack/assets/javascripts/react_app/routes/common/PageLayout/PageLayout.js
Outdated
Show resolved
Hide resolved
| )} | ||
| </ToolbarItem> | ||
| {isLoading && ( | ||
| <ToolbarItem id="toolbar-spinner"> |
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.
| <ToolbarItem id="toolbar-spinner"> | |
| <ToolbarItem id="toolbar-spinner" alignSelf="center"> |
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.
Good catch, thanks!
984e6ea to
1e83211
Compare
Visually nothing should change, unless the toolbarButtons are PF3 and there are more than 1 of them, which from what I saw is only the tasks table.
tasks table buttons will get 0 space with this PR, but it will be fixed in a follow up PR in foreman-tasks
PageLayout is used in tasks table, audits list, Filters Form,
RegistrationCommands, UpgradePage and other plugins