zulip
zulip copied to clipboard
organization_settings: Add dropdown to filter users list by role.
- [x] Adds a dropdown in the
organization settings>users list
page to filter the users list by role. - [x] By default the user_list will be sorted according to the role, from highest permissions to lowest permissions (Owners, Administrators, moderators, members, guests).
- [x] Dropdown works for different languages also.
Fixes: #18617
Screenshots and screen captures:
Users list header:
- Before:
- After:
- Narrow Screen:
Filter users list (demo):
-
English:
-
Other language:
Default users list:
- Difference from previous plan:
The issue description said to implement a
multiSelectDropdown
, however, we have not tested themultiSelectDropdownList
properly. So, we can just go ahead with thesingleSelectDropdown
for the first iteration and then migrate to multi once it has been tested properly.
Self-review checklist
- [x] Self-reviewed the changes for clarity and maintainability (variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
- [x] Explains differences from previous plans (e.g., issue description).
- [x] Highlights technical choices and bugs encountered.
- [x] Calls out remaining decisions and concerns.
- [ ] Automated tests verify logic where appropriate.
Individual commits are ready for review (see commit discipline).
- [x] Each commit is a coherent idea.
- [x] Commit message(s) explain reasoning and motivation for changes.
Completed manual review and testing of the following:
- [x] Visual appearance of the changes.
- [x] Responsiveness and internationalization.
- [x] Strings and tooltips.
- [x] End-to-end functionality of buttons, interactions and flows.
- [x] Corner cases, error conditions, and easily imagined bugs.
Hello @zulip/server-settings members, this pull request was labeled with the "area: settings (admin/org)", "area: settings UI" labels, so you may want to check it out!
Hey @alya , I have made the suggested changes. However, the dropdown works in all languages, but the titles inside the dropdowns are not changing with respect to the language. I am working on that part CZO .For the rest, could you please review? Thanks.
Thanks! The issue suggested a multi-select dropdown. Is that difficult to implement? If so, the current approach seems OK.
@alya , as aman quoted on CZO:
MultiSelectDropdownList is actually new and not used anywhere in the codebase right now. you can try using DropdownListWidget for now and we can iterate on it later when MultiSelectDropdownList has been tested properly.
We have not tested the multiSelectDropdownlist
properly, so we can just go ahead with the singleSelectDropdown
for the first iteration and then migrate to multi
once it has been tested properly.
I have made all the changes, could you please review the PR? Thanks.
We have not tested the
multiSelectDropdownlist
properly, so we can just go ahead with thesingleSelectDropdown
for the first iteration and then migrate tomulti
once it has been tested properly.
Got it. Please add a note in the PR description. I unchecked this point in the self-review checklist for you. ;)
Explains differences from previous plans (e.g., issue description).
The alignment of the dropdpown and filter fields isn't right when one of the users has a really long name:

Looks good to me otherwise!
The alignment of the dropdpown and filter fields isn't right when one of the users has a really long name:
@alya , I'm unable to reproduce this bug, even when using very long names. Could you please provide a GIF or outline the steps to recreate this issue?
For me it looks like this:
do you need any help in this?
do you need any help in this?
No thank you, I will let you know whenever I want to :)
Hm, it seems to be unrelated to long names. This is what I'm seeing; I really haven't taken any steps other than opening the Users
menu.

@alya , I have fixed the issue and updated the PR description accordingly, could you please review it again? thanks. :)
Hrm, opening the Users panel as aaron
, I see this vertical alignment issue:

In a narrow window, I think we want less vertical space between Users<->All roles and All roles<->Filter users, and more space between Filter users and the table.

Hrm, opening the Users panel as
aaron
, I see this vertical alignment issue:
I am not sure why this issue occurred. One possible reason could be that we recently changed the logic of CSS to have a symbol before text, and this PR was updated before that. I was using tag names for the CSS, but I have made all the suggested changes, and now it is looking as expected. Please let me know if you still see any issues.
In a narrow window, I think we want less vertical space between Users<->All roles and All roles<->Filter users, and more space between Filter users and the table.
For most of the settings table heading, we are using Bootstrap, which sets a 10px margin-top-bottom for h3
tags (heading) by default. To maintain consistency between the heading and filters, I have made changes so that all the sections (heading <-> role filter <-> search filter <-> table) have a 10px gap. Please let me know if you want me to reduce the gap further.
@alya , could you please review the PR again, Thanks :)
Looks good to me! @sahil839 could you please review this one?
@sahil839 , I have made all the suggested changes. As for refactoring the list_widget
code to use only a single function, I will work on it after receiving feedback from @timabbott.
@timabbott Marking for integration review for you to take a look, but see @sahil839 's question above regarding the overall approach.
Though, I am not sure about the approach here. I think we should only have a single filter function and it should consider all the filters available for the table - in this case both search input and the role dropdown rather than passing filter and then role_filter separately. To support this, careful restructuring of the list_widget code would be needed. @timabbott thoughts?
Yeah, I agree, we should have a single filter function.
Heads up @palashb01, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main
branch and resolve your pull request's merge conflicts accordingly.
Hello @palashb01, If you agree, I'd like to collaborate with you and finish up the work on this feature and we could Co-Author the commits.
Hello @palashb01, If you agree, I'd like to collaborate with you and finish up the work on this feature and we could Co-Author the commits.
Hey sure, you can go ahead and co-author this feature :)
Closing in favor of #29597 for this issue. Thanks @palashb01!