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

Fix pagination for Users page #1846

Closed
6 of 7 tasks
bajiat opened this issue Oct 31, 2016 · 18 comments
Closed
6 of 7 tasks

Fix pagination for Users page #1846

bajiat opened this issue Oct 31, 2016 · 18 comments
Assignees
Milestone

Comments

@bajiat
Copy link
Contributor

bajiat commented Oct 31, 2016

The Users page is used for adding admin rights to users and deleting users. The page is only available for admins.

Currently it is not possible to view all users if the number of users exceeds one page, since there is no pagination. Add pagination functionality to Users page. Also, total number of users could be useful informatio, for example, as part of pagination: "showing 1-20 users of 325 users"

Consider either fixing the package or finding an alternative one.

Definition of done

  • Admin is able to view all users registered to a deployment
    • e.g. by scrolling through pages
  • Admin can see total number of users
  • Optionally, UI texts have been internationalised.
  • Optionally, name field shows the user name (if user has given one)
    • If user has no user name, Name field shows email.
    • Label for Name field should be user name.
@bajiat bajiat added this to the Sprint 34 milestone Oct 31, 2016
@marla-singer marla-singer self-assigned this Oct 31, 2016
@Nazarah
Copy link
Contributor

Nazarah commented Nov 1, 2016

suggesting to the following:

  1. using standard table style (one used in dashboard)
  2. Placing the action buttons at right side of each table row.
  3. dropdown to view no. of entries in a table at a time
  4. pagination button in top and bottom part of the table.
  5. Currently roles don't appear in the user-page, so should we make this field mandatory while adding the users?

@brylie
Copy link
Contributor

brylie commented Nov 1, 2016

@marla-singer we have to consider whether 'font-awesome' is a dependency of the 'account-admin-ui' package. If we want to use FontAwesome icons in the 'account-admin-ui' package, then we need to add FontAwesome as a dependency in the package.json.

What is your preference?

@marla-singer
Copy link
Contributor

@brylie As for me they are the same, I don't have any preference in the fonts

@marla-singer
Copy link
Contributor

marla-singer commented Nov 2, 2016

@Nazarah Which values for dropdown list do you suggest? I thought about 25,50,75,100.

@brylie
Copy link
Contributor

brylie commented Nov 2, 2016

@marla-singer OK, lets keep our dependencies simple then. Our users table already relies on Bootstrap, so GlyphIcons are directly available.

@bajiat
Copy link
Contributor Author

bajiat commented Nov 2, 2016

@Nazarah @marla-singer I think it would be enough to have one sensible default for number of users shown on the page. No dropdown to select nr of users required. This page is only accessible for admins. (Unless you have already started to implement this.)

@marla-singer
Copy link
Contributor

@bajiat Okay, no one dropdown list 👌

@marla-singer
Copy link
Contributor

@brylie Could you please give me advice how i can implement internationalization in package to use it after in APINF project?

@marla-singer
Copy link
Contributor

@bajiat Now It looks like this:
joxi_screenshot_1478089250859 1

@bajiat
Copy link
Contributor Author

bajiat commented Nov 2, 2016

@marla-singer Looking good! Great to have both pagination and total number of users. Also makes sense to have actions on the right side.

@brylie
Copy link
Contributor

brylie commented Nov 2, 2016

Could we show the username under 'name' instead of email here? That would close a related issue #909

@marla-singer
Copy link
Contributor

@brylie Yes, we can.
We have field "username" in users collection
https://github.com/apinf/platform/blob/develop/users/collection/users_schema.js#L17
But the table shows values from profile.name field.
https://github.com/brylie/meteor-accounts-admin-ui-bootstrap-3/blob/master/client/accounts_admin.html#L33
To fix it these is two cases:

  1. We restructure our users collection in APINF project: copy value from username field to profile.name
  2. I change the condition in package by hand: replace all profile.name to username

I learnt Meteor Docs about users colletion: http://docs.meteor.com/api/accounts.html#Meteor-users
And it is logical to fix like second case. Because username storages unique user's name (login) and it's obviously to use it.

What do you think?

@brylie
Copy link
Contributor

brylie commented Nov 2, 2016

I would recommend simply showing the username field directly:

{{# each users }}
 ...
  <td>
    {{ username }}
  </td>
...
{{/ each }}

This is for two reasons:

@bajiat
Copy link
Contributor Author

bajiat commented Nov 2, 2016

@marla-singer User name was an optional item in definition of done, but it's great if you can implement that as well.

@marla-singer
Copy link
Contributor

marla-singer commented Nov 3, 2016

@brylie PR is ready https://github.com/brylie/meteor-accounts-admin-ui-bootstrap-3/pull/1

TODO:

  • Add information about new package version

@brylie
Copy link
Contributor

brylie commented Nov 3, 2016

how I can point dependencies in prepared package?

@marla-singer, you can use api.use('package-name', 'client'), for example to include 'package-name' in 'client' code.

See Meteor package.js docs

@marla-singer
Copy link
Contributor

@brylie I pointed dependencies

@marla-singer
Copy link
Contributor

@bajiat The PR was merged. apinf/meteor-accounts-admin-ui-bootstrap-3#3
The next steps are publish package to atmosphere and include it on APINF project

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

No branches or pull requests

4 participants