Skip to content
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

Desktop: Use the react-select library for tag input #1589

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

CalebJohn
Copy link
Collaborator

@CalebJohn CalebJohn commented May 27, 2019

I finally found a stable input that we can use for tags, react-select
It's very flexible which means we can change the behaviour however we like.

One thing I was sure about is deleting tags with backspace.
Currently the user cannot delete tags by pressing backspace while the cursor is next to an item. I intentionally disabled this to prevent users from accidentally deleting tags. Personally I would prefer to delete using backspace, so I thought I would reach out for opinions and see what others felt would be the most sane behavior.

image

image

edit: fixes #1535

@tessus
Copy link
Collaborator

tessus commented May 27, 2019

I think deleting tags with backspace is fine. It's not that you delete the tag, but remove it. Thus re-adding it will not be an issue, if a user accidently removes it.

@tessus tessus added the desktop All desktop platforms label May 28, 2019
@laurent22
Copy link
Owner

I also think that using backspace for deletion is fine.

It's nice that you found a lib we can use for this. I'll try your pr soon.

@CalebJohn
Copy link
Collaborator Author

Okay, I've enabled backspace to delete tags. I anticipate that there will be a few complaints about it, but I agree that the majority should appreciate it.

@laurent22
Copy link
Owner

Looks good overall, just a few things I've noticed:

  • Center text vertically between "Add or remove tags" and dropdown list.
  • We've lost the ability to press ESC to exit the dialog box
  • Tags need to use theme font
  • Ideally the cross to delete a tag should be white (theme.color) when the dark theme is used, but if it's too complicated it's fine in black.

If it's too complicated to customise the tag CSS (i.e. if we need to add special CSS files or rules to theme.js) then just ignore the last two comments.

@CalebJohn
Copy link
Collaborator Author

All done! I didn't think the css tweaks were possible but when I looked again it was pretty easy to do.

@laurent22
Copy link
Owner

It works great now, thanks a lot @CalebJohn!

@laurent22
Copy link
Owner

By the way, it's fine to have multiple commits on a pull request. In the end I always do a "Squash and merge" anyway so there will be only one commit on master.

@laurent22 laurent22 merged commit 799ad5f into laurent22:master Jun 7, 2019
@CalebJohn
Copy link
Collaborator Author

Thanks @laurent22 that's good to know.

@CalebJohn CalebJohn deleted the react-select branch June 8, 2019 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request Please add tags list to Windows Desktop
3 participants