-
Notifications
You must be signed in to change notification settings - Fork 289
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
Search images in registry in Image Pull form #7928
Conversation
it won't be a required changes but IMHO it will great to have search as long as we type the values and keep the button being 'pull' so you can just use what you type but also when you're typing something you see the suggestion and you can use it (same pattern as google search) |
Love the capability, but same comments as @benoitf about the UI. Maybe separate PR for renderer so we can merge everything else while we iterate? Personally I'd like it to automatically search if I pause for a moment, and beneath the input it either confirms you've typed a valid name or shows a list of matches you can click on. This pattern is probably general/reusable enough that we will need an 'AssistedInput' at some point. I suppose that means I'd prefer keeping it a very simple UI here, maybe somewhere else we add a more explore/browse UI using the same API. |
I was not sure of the UX for the search, so I have done the simplest (for me) to be able to start the conversation about it (I have added a topic for next UX call). I have created a PR #7930 for the backend part, so we can merge the backend in the meantime |
b3d9fa8
to
609062d
Compare
--input-padding="0" | ||
--item-hover-bg="{colors.get('dropdown-item-hover-bg')?.value}" | ||
--list-background="{colors.get('dropdown-bg')?.value}" | ||
--placeholder-color="{colors.get('input-field-placeholder-text')?.value}" |
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.
is there any particular reason you are getting the colors from the colors store ?
The following syntax is possible and would makes it easy to migrate to the ui library if we decide to include this components.
--placeholder-color="{colors.get('input-field-placeholder-text')?.value}" | |
--placeholder-color="var(--pd-input-field-placeholder-text)" |
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.
ah, thanks, this is much better this way; I have made the change
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.
I am seeing some styling issues and odd behaviour:
- No bottom border or rounded outline to start, doesn't match our Input or other controls / not clear to click on it.
- When I click on the input the popup area appears immediately.
- Again, styling doesn't match existing popups, looks more like a disabled button.
- If I start to type and then click outside of the input, it deletes whatever I've typed.
Screen.Recording.2024-07-15.at.2.21.45.PM.mov
6e4cdab
to
98aa560
Compare
I have made some changes to the styling. There is still a problem with the initial focus. The focus is initially set for the select, but is removed after a second or so. I opened an issue rob-balfre/svelte-select#698 |
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.
The styling is getting better. It still doesn't feel like one of our other controls (mostly the padding difference) but is closer. Nit - selection outline on the X is blue.
I'm still finding the behaviour a bit odd, e.g.:
- if you type
docker.io/mongo
it still says 'Please enter a value'. You have to pick that option from the list before the warning goes away. - Picking an option adds the stars to the entry field like
[★10306] ✔
. - Once you select an option, you can't edit. e.g. if you accidentally select the wrong one you need to start typing or click X and start over, which isn't obvious.
I don't know if I'm setting the bar too high, or this will be a long cycle of trying to force it to have the expected look/behaviour. 🫤
For most of these points, this is the expected behaviour of the component. If we try to change this behaviour, the result can be very hacky. If we want to have an opinionated behaviour for this component, is seems that it will be easier to rewrite it. WDYT? |
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.
LGTM but shouldn't we favor quay.io rather than docker.io 😡
I changed the styling, to adjust the height of the input, and the selection outline. |
2c82ff1
to
71f76e2
Compare
bfe2a41
to
b3bc8fa
Compare
When trying to fix e2e tests, I realize that the user cannot select a specific tag of an image... |
The search endpoint of some registries do not work as expected, for example quay.io which is not able to find an image by giving its org repo (here org=podman-desktop-demo and repo=podify-demo-backend):
but:
The same search works on docker.io:
|
Yes I think so. I'll work on this |
@axel7083 For the moment, to be able to list tags for private repos, you will need to add a registry I have added a commit to add to the Select items the image name entered by the user, when it is not returned by the search, so the user can pull any image |
da11f42
to
069e59c
Compare
The ability to search images and see all the tags is awesome, but while testing I'm still noticing minor issues with the UI:
I'm being very picky, but despite the excellent work @feloy has done to make it fit in the component just isn't feeling super usable or natural to me. I'm happy to keep reviewing and if others feel it's ok I'm not going to block, but also wondering if we should discuss other options. |
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
use the same color as non disabled, to stick with what the Input component is doing Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
069e59c
to
190a713
Compare
Signed-off-by: Philippe Martin <[email protected]>
One problem highlighted by e2e tests: If an image has more than 100 tags, only 100 tags will be returned, and the one wanted by the user may not be in the list. In this case, there is no way for the user to specify a non-listed tag |
Following today's discussion, I have found this component https://github.com/metonym/svelte-typeahead which behaves more like we would like (just copy the content of the selected entry in the input, without having to clear the entry with the X to continue to edit it). The problem is that is depends on the (edited to copy-paste the correct components) |
I'm trying this with a private registry I use (hosted on forgejo locally), and I'm getting some odd issues. The first is a 401 error in the console which is expected, but there was no warning in the UI:
|
I am also getting multiple "An object could not be cloned" errors in the console:
|
Closing in favor of #8786 |
Signed-off-by: Philippe Martin [email protected]
What does this PR do?
Search images in registry in Image Pull form
Two fields:
Screenshot / video of UI
search-image.mp4
What issues does this PR fix or reference?
Fixes #971
How to test this PR?
Go to the Images list page, press Pull button, then search images (see video)
e2e tests have been modified to support the new system. A new parameter has been added to
pullImage
, two parameters are now necessary:searchTerm
: the text entered in the search inputimageName
: the item selected in the result list