-
Notifications
You must be signed in to change notification settings - Fork 123
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
Generic UI list editor #3531
Generic UI list editor #3531
Conversation
1a391f5
to
e3e4c13
Compare
Thanks for fixes, Ebu! Some minor design issues:
|
1, 2, 3 fixed, yay! Ad. 4 I don’t think it works correctly now. When I click Add people, an editable fields appears that contains values from all selected images and allows adding a new value that will apply to all of them. But:
While such an editable field (after fixing all above problems) would indeed be a more powerful editing device, allowing for adding to all something that exists only on some images, editing the name of something being added only to some images, removing something from any etc, I think this is not a good UI device for the Browser (especially because now it’s for People only) for the following reasons:
I think we should (at least for now), just make Add people plus button behave like in Labels: present a user with an empty field for adding a new value(s) to all selected images. That’s it. The other pills (and values) would stay on selected images as they were before the edit, just the new value(s) would get added. Buttons on pills already allow removing from any/all and adding to all for any existing pills. So we will miss only editing from the above approach. For which, the workaround would be to add a new correct value and remove the old incorrect one. This is also way easier, I guess. If anyone disagrees, please shout! Ad 5, 6 They are still broken for me in latest Firefox and Chrome: removing a person from multiple pictures causes progress bar to go haywire and doesn’t work. As always, please let me know if anything isn’t clear or if you are seeing different behaviour. Or, obviously, if you have different opinion on how stuff should work/look! |
the progress bar works in normal add and doesn't work on partial add, I'm looking at it now |
1ec938f
to
9c04d06
Compare
Hi @paperboyo |
…nstances of labels, keywords and people in image to generic component
b7048b7
to
414b6a6
Compare
Hi. Thanks for the latest fixes, coming along nicely! Ad 4 This is still the case. Let me know if you (or anyone) disagrees about the proposed simplest route in this comment above. |
489d9c0
to
ca8368a
Compare
Seen on cropper, kahuna, media-api, metadata-editor (created by @adrielulanovsky and merged by @andrew-nowak 10 minutes and 58 seconds ago) Please check your changes! |
Seen on usage, collections, image-loader, image-loader-projection, thrall, leases (created by @adrielulanovsky and merged by @andrew-nowak 11 minutes and 3 seconds ago) Please check your changes! |
Seen on auth (created by @adrielulanovsky and merged by @andrew-nowak 11 minutes and 11 seconds ago) Please check your changes! |
What does this change?
This creates a generic component for displaying and editing lists of strings, such as labels, people in image, keywords, etc.
How can success be measured?
Previous behaviour in fields that contained arrays of strings is maintained. When trying to display a new field which is an array of strings, the implementation should be straightforward using this component.
TBD: Do we want to maintain the behaviour and look of the people in image tokens? The generic component allows for the possibility of adding and deleting single tokens from all selected images. If we want to do that for people, then we should change the style to something similar to what Labels has, so we can add a + and an x to add / remove. I left the same style as the labels, but this can be changed accordingly.
Screenshots
Who should look at this?
@guardian/digital-cms
Tested? Documented?