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

Convert to server-side query: admin panel -> edit user group -> search users #331

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

echappen
Copy link
Contributor

@echappen echappen commented Mar 6, 2025

Pull Request Checklist

Closes #167

Changelog Entry

Description

For admin panel -> users -> groups -> edit user group -> search users:

  • Removes typeahead search in favor of server-side search
  • This will make user search more resilient at scale, as the users list was previously retrieving every single user in the database
  • Also capitalizes other search button for general users list, to be consistent with the casing of other buttons throughout the app

How to test

  1. Log in as an admin
  2. Go to admin panel
  3. Click Groups, then click the name of a user group
  4. In the Edit User group modal, select the Users tab
  5. Try searching for a non-admin user (admins are filtered in keeping with existing functionality)

Changed

  • users search now uses a button

Removed

  • Removed client-side filtering of users in favor of server-side query

Security

  • Note: No change to the API query method itself, we're using the same one as before

Breaking Changes

  • None known

Additional Information

I think having the search button as text is important for accessibility. However, replacing the magnifying glass icon with a button makes this top area of the modal a little awkward with the current design patterns. I think the awkwardness comes from having no borders around elements. I'm very open to any thoughts on how to make this UI better. I've included some potential design options as screenshots below

Screenshots or Videos

This is the original modal:

Screenshot 2025-03-06 at 10 45 02 AM

These are design options I considered:

Screenshot 2025-03-06 at 10 44 54 AM

Screenshot 2025-03-06 at 10 57 50 AM

Screenshot 2025-03-06 at 10 56 58 AM


This is the option I went with that's currently coded:

Screenshot 2025-03-06 at 10 56 58 AM

@echappen
Copy link
Contributor Author

echappen commented Mar 6, 2025

@actuallyjenn Tagging you on this for a UI review. See PR description for details.

Copy link
Contributor

@jfredrickson jfredrickson left a comment

Choose a reason for hiding this comment

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

LGTM in terms of functionality/code, will defer to @actuallyjenn on the UI question. IMO, agreed that this app's UI has some delineation awkwardness.

Copy link

@actuallyjenn actuallyjenn left a comment

Choose a reason for hiding this comment

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

looks good to me from a UI perspective. we'll have to go back and figure out how to handle some of these elements better later but appreciate the different options you included eleni! i'm thinking we'll probably add borders in the future.

@echappen
Copy link
Contributor Author

echappen commented Mar 7, 2025

Thank you both!

@echappen echappen merged commit 4745182 into main Mar 7, 2025
4 of 5 checks passed
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.

Figure out what parts of UI fall apart at scale?
3 participants