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

Add preview seleted items #10

Merged
merged 2 commits into from
Feb 20, 2019
Merged

Conversation

ahmedkandel
Copy link
Contributor

->showPreview() option to show checkbox which toggles "only selected items" preview.

->showPreview() option to show checkbox which toggles "only selected items" preview.
@dillingham
Copy link
Owner

dillingham commented Feb 18, 2019

@ahmedkandel this is great! Very useful addition.. thank you for the PR

What do you think about

Keeping the header and search when previewing
Allow for easy deselecting & wouldn't jump as much when toggling

Maybe showSelected instead of showPreview
I like preview, just wondering if "selected" might be more universal 🤔

Thanks again 😄

selected

@ahmedkandel
Copy link
Contributor Author

@dillingham me who have to thank you for the great packages.

I think the wording "Selected or Preview" is somehow connected to the "Toolbar & Search",

Preview approach "like GitHub comment preview" is valid to have a final look before saving and make sure you selected the correct items otherwise deselect wrong ones. while for the toggling jump we could use an overlay as following:

screenshot_2

screenshot_1

Show (Only) Seleted approach is valid as a filter but may interfere with toolbar search and select all functionality. I think will confuse the user.

May we use the wording combined as following:

screenshot_4

What do you think?

@dillingham
Copy link
Owner

Ok I agree. I like the second option "Selected Items (3)" but with Preview [ x ]
We can always offer a lang file option to change it later.
Keeping it as preview is good because help text may run longer.
I also like "Selected Items" for when it comes time to translate this in a lang file.
I think the header is a good way to keep the header height from making it jump too 👍

@ahmedkandel
Copy link
Contributor Author

Good, will make a new commit with header overlay "Selected Items (n)" and keep the checkbox as "Preview", both will be translatable in order to offer language file later on.

Also will fix the following scenario: when deselecting an item in preview mode the select all checkbox will be activated as it considers all items are selected which is not true.

add preview header overlay to cover toolbar, make strings translatable, fix selecting all fake activation after deselecting items in preview mode.
@dillingham dillingham merged commit da60ea5 into dillingham:master Feb 20, 2019
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