-
Notifications
You must be signed in to change notification settings - Fork 510
Include tags in editable list #6615
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
Include tags in editable list #6615
Conversation
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 like it! I just have some nitpicks;
I'm not a huge fan of that indicator tbh :/
It feels a bit too "heavy", especially compared to the text ellipses we have in other columns. Maybe it could be a bit less prominent?
I'd also add some vertical gap to the tooltip tags:
Could also be nice to sort the tags, the order seems to change with every page refresh
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
1315303 to
952c975
Compare
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.module.scss
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
7ce0722 to
4d03741
Compare
4d03741 to
023a6c6
Compare
765bfca to
0889ae1
Compare
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Show resolved
Hide resolved
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
|
I think this was nearly done besides the stuff from my last review... maybe worth reviving so it can be merged soon? :) |
0889ae1 to
7660e07
Compare
|
I forgot about this PR, sorry. I just finished up correcting the latest comments. |
indico/modules/events/editing/client/js/management/editable_type/EditableList.jsx
Outdated
Show resolved
Hide resolved
3931f42 to
52c0e83
Compare
52c0e83 to
80d1983
Compare
Refining ui tags class name
It doesn't do anything useful...
80d1983 to
db6ad22
Compare
13bf06c to
2ab3a1e
Compare
It contains the same data as what's already in the cell
7c1ee9f to
943cf7d
Compare



Add a column to include the editable tags if any exist

closes #6614