-
Notifications
You must be signed in to change notification settings - Fork 0
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
Walter/tag picker #105
Walter/tag picker #105
Conversation
… into walter/tagPicker
…both dismissal calls to be triggered when clicked outside TagPicker's primary dropout
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.
So I have spotted two things that I think we may need to consider.
The first is in the cardDetailView, the user cannot add/edit the tags before edit mode is engaged.
In this view, the "X" button to disassociate the tags should be present when the mouse is hovering over it. And the add/edit tag button should be appended at the very end.
I think it is better to have the editing tag functions separate from the editing cards.
The second thing I found was that in tagPicker, the tag will disappear from the list of tags once you have associated it with the card. With this implementation, the user won't be able to edit the tags (e.g. change colour) unless they first disassociated.
Other than that, very nicely done.
Another issue I encountered is when I go tag picker --> associated any tag/s --> edit tag --> click out to close tag edit, the state will be reverted back to before I associated any tags. |
a more general description would be any edit to any tags would override state of cardDetail. I believe #108 fixes this. |
I'll be testing this shortly using #108 |
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.
Tag's components should have their own sub folder like cardDetails' components do imo.
other than that lgtm
yes sir |
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.
Mostly code style changes.
Closes #84 .
To test the ui, please spin up a local server and test the ui.