← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~ilasc/launchpad:oci-recipe-push-rules-edit into launchpad:master

 

Thanks for the changes, ioana! They are looking better now. Just a few more points:

- On the UI, visually the checkboxes do not behave that much like radio buttons. I can click and select both boxes at the same time, for example. It might work if you use them as Choice + Vocabulary instead of separated booleans, but I have to admit that I didn't test it. Maybe a simple custom Javascript to at least unselect the items could make this a bit better. Something more or less like this:

```
a = document.getElementById("field.use_existing_credentials");
b = document.getElementById("field.add_new_credentials");
a.onclick = () => { if(a.checked) b.checked = false };
b.onclick = () => { if(b.checked) a.checked = false };
```

- Now that the link to edit credentials is at the top, maybe we could put the "delete" checkbox a bit to the left. It is too separated from the image name now.

- Another thing that I'm missing is the credentials on the edit table. Maybe putting the credential description alongside with the image name would be nice?

- For the existing credentials, we are showing something like `https://registry.dockerhub.com pappacena`. Since the username is usually smaller, putting it after the URL looks a bit like it's part of the URL if you do not pay attention while reading. Maybe we should format it like `"pappacena" user at https://registry.dockerhub.com` or something similar.
-- 
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/386371
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:oci-recipe-push-rules-edit into launchpad:master.


References