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

selectAll() Select only visible countries #197

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bab9e9
Copy link

@bab9e9 bab9e9 commented Jun 4, 2020

This is to satisfy issue, "selectAll should select all visible regions". Which I created.
I'm pretty new to Github, so I am not really sure about the protocol for getting changes accepted.

@netlify
Copy link

netlify bot commented Jun 4, 2020

Deploy preview for frosty-benz-8c81bf ready!

Built without sensitive environment variables with commit 064a18a

https://deploy-preview-197--frosty-benz-8c81bf.netlify.app

@MrSpiffyClean
Copy link
Contributor

Checked the deployed preview and I'm not exactly sure I like this new change, because now pressing on Select All ends up deselecting everything else that was before. I think the action that makes more sense with the Select/Deselect All is that they act on the filtered results only, although I would like more feedback on this.

As for when this gets accepted, it's all up to the maintainers, mostly Aatish. Remember that this isn't a full-time job for anyone that contributes and in the interest of not breaking stuff, things move slowly. There's a Contribution Guide you can read if you're interested.

@bab9e9
Copy link
Author

bab9e9 commented Jun 10, 2020

What MrSpiffyClean suggested would be something more like "Add all filtered (search) to selection".
Maybe "Add all these", "Drop all these". With "Add|drop all" in addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants