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

Select - Updates select mechanism, table styling, and adds tests. #15

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

Conversation

ehelms
Copy link
Contributor

@ehelms ehelms commented Jun 3, 2013

No description provided.

<div class="select-all-action-container"
ng-show="selected.moreResults()">
All {{ table.offset }} results shown are currently selected.
<a class="table-select-all-action">Select all {{ table.total }} results.</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering, if it makes sense to say user that he has selected all options and give him option to "select all". And if I deselect one row, this options disappears. It looks that it needs to be vice-versa - when I don't have selected all, I have option to actually select all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happens when I check all (from the left top) - the message says: "All results shown are currently selected. Select all results."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This action was intended from this scenario

  • I have 50 of 100 results showing
  • I clicked the 'select all' checkbox, which selected all 50 visible to perform some action
  • Maybe I really want to perform that action on all 100

This link appears and gives them that option. It's similar to how gmail handles when you select all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this makes sense. Actually the sentence "Select all 100 results" as is done in the code would help, however it seems that total number is not working in the example.

@jcoufal
Copy link
Member

jcoufal commented Jun 7, 2013

Smaller comments inline, after fixing these ACK.

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.

2 participants