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

Fixes #35763 - Switch to standard PF4 search input in Katello #10401

Merged
merged 1 commit into from Feb 7, 2023
Merged

Fixes #35763 - Switch to standard PF4 search input in Katello #10401

merged 1 commit into from Feb 7, 2023

Conversation

lfu
Copy link
Member

@lfu lfu commented Jan 5, 2023 •
edited

What are the changes introduced in this pull request?

Katello is switching to Foreman's standard PF4 search input.

Considerations taken when implementing this change?

Depends on theforeman/foreman#9571

What are the testing steps for this pull request?

Check all the search field work well in new host details pages and content view components.
Verify this PR fixes https://projects.theforeman.org/issues/35988.

@theforeman-bot
Copy link

Issues: #35763

@lfu lfu force-pushed the searchbar_pf4_35763 branch 8 times, most recently from a6b206b to cdee1ec Compare January 12, 2023 15:52
@lfu lfu force-pushed the searchbar_pf4_35763 branch 2 times, most recently from 570ed89 to 0f242cd Compare January 25, 2023 17:58
jeremylenz
jeremylenz reviewed Jan 27, 2023
View reviewed changes
Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Thanks @lfu

First impression is that this looks great! More comments coming soon, but for now I'll just say I love that we were able to remove the searchDelay and autoSearch scopes from all the tests.

Now that we don't use those settings anymore, I think we should also remove the autosearch_while_typing and autosearch_delay settings themselves. (it's in plugin.rb.)

@lfu
Copy link
Member Author

lfu commented Jan 27, 2023

Now that we don't use those settings anymore, I think we should also remove the autosearch_while_typing and autosearch_delay settings themselves. (it's in plugin.rb.)

I have not deleted anything since the Katello search field is being used by some other components.

@jeremylenz
Copy link
Member

I have not deleted anything since the Katello search field is being used by some other components.

Where is it still used? Shouldn't we use Foreman's everywhere?

@lfu
Copy link
Member Author

lfu commented Jan 30, 2023

Where is it still used? Shouldn't we use Foreman's everywhere?

SubscriptionsToolbar and redhatRepositoriesPage.
Should they be changed as well? Thought this change is only for new host details pages.

@jeremylenz
Copy link
Member

SubscriptionsToolbar and redhatRepositoriesPage.
Should they be changed as well?

Ideally yes, we should standardize everywhere!

@lfu lfu force-pushed the searchbar_pf4_35763 branch 4 times, most recently from b321a8b to 1b3debf Compare February 3, 2023 17:45
jeremylenz
jeremylenz approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Great work @lfu !

Let's continue on in the next PR. ACK 👍

@lfu lfu merged commit 1ecd0f2 into Katello:master Feb 7, 2023
@lfu lfu mentioned this pull request Mar 9, 2023
Merged
@lfu lfu deleted the searchbar_pf4_35763 branch November 16, 2023 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremylenz jeremylenz jeremylenz approved these changes

Assignees
No one assigned
Labels
Not yet reviewed
Projects
None yet
Milestone
No milestone
3 participants
@lfu @theforeman-bot @jeremylenz