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

New features for admin users list #2998

Closed
wants to merge 9 commits into from
Closed

Conversation

davwheat
Copy link
Member

@davwheat davwheat commented Jul 31, 2021

Fixes #3742

Changes proposed in this pull request:

Add new features to the ACP users list:

  • First/Last page buttons
  • Ability to go direct to a page number
  • Permalinks to any page via ?page=xxx, and button to copy a permalink to the current page
  • Add display name column
  • Add email confirmation status and toggle (requires Add ability to set email confirmation state to false #2997)
  • User creation
  • Search
  • Sorting

Reviewers should focus on:

Screenshot

4uCxmzgf.mp4

Confirmed

  • Frontend changes: tested on a local Flarum installation.

@davwheat davwheat self-assigned this Jul 31, 2021
@davwheat davwheat added this to the 1.1 milestone Aug 1, 2021
@davwheat davwheat force-pushed the dw/2988-user-list-upgrades branch from 536a41f to 477b8e1 Compare August 1, 2021 12:32
@Ralkage
Copy link
Contributor

Ralkage commented Aug 7, 2021

LOVE IT ♥ for the table column names, could we have it so that the first letter of each word is capitalized? (e.g. Display Name, Email Confirmed, etc).

Edit: I would also suggest doing the same for the rest of the text on that page including the modal title to keep with the same consistency as the rest of the forum.

Examples:
Create New User
Go to Page
Create Link to Page 45

@askvortsov1
Copy link
Member

This PR is getting a bit large. Maybe let's do searching and sorting in follow-ups?

@davwheat
Copy link
Member Author

Good idea. I don't think I'll have time to do them soon anyway with the accessibility issues.

Some of the changes (itemlists) make it easier than ever for an extension to modify the userlist to add search and sort, too :P

@davwheat davwheat marked this pull request as ready for review August 15, 2021 15:50
@clarkwinkelmann
Copy link
Member

I have not followed the progress here but if I may make a suggestion, could we make the display name column into the link to the user profile?

I'm creating an extension that hides the username from everywhere, so in my extension I added a display name column that becomes the link. I see this PR will add the same column, but it wouldn't be a link.

Or the profile link could be an action of its own instead of being one of the data columns.

@askvortsov1 askvortsov1 changed the base branch from master to dw/adminpage-ts-rewrite August 21, 2021 01:08
@SychO9
Copy link
Member

SychO9 commented Aug 21, 2021

I'm creating an extension that hides the username from everywhere

Btw, there's an issue in core for 1.1 where we will end up having usernames completely hidden from the frontend unless the actor has edit credentials permission (https://github.com/flarum/core/issues/1734).

Base automatically changed from dw/adminpage-ts-rewrite to master August 23, 2021 00:59
@askvortsov1
Copy link
Member

Will review once rebased.

@davwheat davwheat force-pushed the dw/2988-user-list-upgrades branch from 8501868 to d43b72f Compare September 10, 2021 16:05
@davwheat
Copy link
Member Author

Rebased!

@davwheat
Copy link
Member Author

Ugh VS Code formatted the locale yml... On it

@askvortsov1 askvortsov1 modified the milestones: 1.1, 1.2 Sep 28, 2021
@askvortsov1 askvortsov1 self-requested a review October 14, 2021 18:42
Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

What are your thoughts on potentially using a shared "PaginatedListState" for tables, actually paginated lists, and infinite scroll lists? It could be handy, but it could also be overengineering I suppose.

content() {
const fields = this.fields().toArray();

console.log(this.state);
Copy link
Member

Choose a reason for hiding this comment

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

Seems left over

/**
* A Modal that allows admins to create a new user.
*/
export default class CreateUserModal extends Modal {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we could use the same component for the Create and Edit modals?

return items;
}

private twoWayLinkAttrs(key: keyof typeof this.state, valueName: string = 'value', eventName: string = 'oninput') {
Copy link
Member

Choose a reason for hiding this comment

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

This seems overly complex

const indexOfSearch = location.hash.indexOf('?') + 1;
const search = indexOfSearch >= 0 ? location.hash.substr(indexOfSearch) : '';

const params = new URLSearchParams(search);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it might make sense to store this info in params namespaced by a table ID. That way, we could support having multiple tables on the same page, and also keep this logic in the table component.

Alternatively, we could support both modes, but error if more than one table / paginated list tries to use non-namespaced-mode?

@davwheat davwheat modified the milestones: 1.2, 2.0 Dec 21, 2021
@davwheat davwheat force-pushed the dw/2988-user-list-upgrades branch from 0fd5350 to 495e9a5 Compare December 27, 2021 19:10
@askvortsov1 askvortsov1 removed this from the 2.0 milestone Mar 10, 2022
@askvortsov1
Copy link
Member

Hello, thank you for your pull request! In order to speed up future development, keep all work in one place, and take advantage of CI tools that can help us avoid breaking changes, we have moved all Flarum code to a single monorepo at flarum/framework. You can read more about this process in our dev diary. Unfortunately, pull requests can't be carried over to the monorepo, so we have to close all open pull requests. If this pull request is still relevant, please feel free to reopen it over on the monorepo. We apologize for the inconvenience, and hope you will consider contributing to Flarum again in the future.

@Eldenroot
Copy link

Please rebase it :)

@jslirola
Copy link

Have these improvements been abandoned or is it possible to continue the progress of them? I think it would be great to implement them in future versions of the core.

I'm not sure if this is the proper place or the official forum to ask it.

@SychO9
Copy link
Member

SychO9 commented Sep 13, 2022

This is planned to be picked up again, can't say when that could be.

@jslirola
Copy link

Thanks for the quick response, I'm glad to know that this has not been forgotten. :)

@umutcandev
Copy link

Sorry to revive the thread, it's been quite a while since September. Will there be any progress?

@umutcandev
Copy link

Being able to search and sorting would be great in the new version. What do you think?

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

Successfully merging this pull request may close these issues.

Amendments to User Page in ACP
8 participants