Cherry pick breadcrumbs switcher search input to 1.18

This search input feature is part of the initial patternfly design, without it, the breadcrumb switcher is incomplete, plus the breadcrumb switcher may be inefficient in some pages (imagine how bad is to paginate over thousands of hosts).

I’d like to see this feature cherry picked into 1.18, same as I believe the users would.
Share your thoughts :slight_smile:

  • Agree
  • Disagree

0 voters

I really like the feature. Few questions/concerns though - what is the impact on packaging side? Is it well covered with tests? From my limited JS reading skills, it seems there were not many tests added while a lot of code has been changed. I think the decision at the end is on @Ondrej_Prazak but it might help to know the answers and know that it has been successfully tested on 1.18 stable branch. Given where we are with builds, I’d be extra cautios adding features to stable branches.

I also believe many users would like to have this in 1.18 :slight_smile: , however this is a new feature that introduces additional packaging dependencies and is rather large in scope (700+ lines changed). For these reasons, I do not see it as a good candidate for cherry-pick.

in all fairness, its 1. one npm package and 2. out of those 700 lines many of them are snapshots and tests, these are not 700 lines of code. having been part of the team who reviewed this PR + the impact it has (and the fact we don’t even have an RC) I would vote (and I did :slight_smile: to try and get it in

I think this is a fair and important feedback, @amirfefer did you try it on 1.18-stable branch?

100% fair - but if the breadcrumb feature is genuinely broken, or at least not very useful, without this, then we’re probably better including it, rather than having a very sub-par UI for the lifecycle of 1.18

I voted yes for cherry-picking this change. I think that it drastically improves the experience of the breadcrumb switcher and that it should be included. As long as we can make sure that it is thoroughly tested on 1.18-stable then I think the benefit outweighs the risk.

Also, without this change we will see bug reports on the lack of a search so one could frame it as a preemptive bug fix :wink:

The test coverage is good. Out of the mentioned 700 lines about 200 is the code. The rest is snapshots and tests.

The switcher and breadcrumbs work without the search field, but I agree it improves the experience. I lean towards yes, let’s cherry pick it to have the complete functionality in. But it definitely needs to be thoroughly tested with the rest of the 1.18 code.

I did, and it works well with 1.18 branch (there’re some small conflicts).