-
Notifications
You must be signed in to change notification settings - Fork 9.7k
UI: optimize pool searching in /targets page #16567
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
UI: optimize pool searching in /targets page #16567
Conversation
Signed-off-by: Ryan Wu <rongjun0821@gmail.com>
77172d8
to
a810e59
Compare
@juliusv Sorry, I don't mean to be rude to @ you. Could you please take a look at this PR? |
@ryanwuer Oh no it's ok, thanks for pinging me. And thanks, that's an interesting approach to handling this problem! I would challenge it though: it's a slight upside for people who want to search for something completely new, but it makes other use cases harder, such as when I just want to edit what has previously been typed. Either because I unfocused the box before I was finished (then my input text disappears) or because I want to change from e.g. Usually when I need to replace all text, I'm used to double-clicking on the input field or alternatively pressing - to select everything before typing, which then automatically overwrites things. So that feels like more of a minor hurdle to me. There is one other alternative though: adding a "clear" button to the selection box, which Mantine also natively supports, see: https://mantine.dev/core/select/#clearable. How about that? It's not a huge upside because you still need to click that first and then enter the new text, but maybe an ok middle ground? |
@juliusv pls take another look of this issue |
@ryanwuer Thanks for the reality check about the input width. I'm still not clear on what the best overall approach is. Just not making the previously inputted text available for textual editing also doesn't seem optimal, even if you need a few steps to be able to edit the suffix. Pressing the Still, so far I still feel like just adding a "clear" button but still letting the user edit the existing text would be the best compromise for now. |
@juliusv I've checked the same page in v2 version. It obviously uses a different UI library, but it clears the inputted texts. For me, it's quite rare to edit the existing texts to do search. When i want to select like No.2 of a ServiceMonitor, i just type the core part of the name, like 'kubelet', and all the options are showed immediately, quite smoothly. |
Ah, interesting. Then at least it would not be a regression, although otherwise we are free to do things very differently from the old UI of course :)
Ok, in the end you're a real user, while I'm not really. Ultimately I don't feel too strongly, I just wanted to make sure that we're not accidentally making things worse for other users. @Nexucis what do you think, should we just merge it and see if anyone complains? |
@Nexucis hey, what do you think? |
I would say if there's no opinion by @Nexucis by tomorrow, let's just merge if then. We can always change things again :) |
Cool. Appreciate it. |
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.
Time has passed, let's merge it :)
yeah sorry for not answering. I was too busy ...
I am often thinking that, specially if we think what is proposed should improve the situation for 90% of the use cases. That's good you didn't wait me to merge this one. Really cool to see these changes |
When searching in pool selection form, users need to clear all texts before inputing search text, which is not very convenient.
Old way:

New way:

Ant UI component library provides the function natively, https://ant-design.antgroup.com/components/select
