-
Notifications
You must be signed in to change notification settings - Fork 887
refactor(site): Improve workspaces filtering #7681
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! This is a huge improvement to our current filtering.
I noticed a few things.
-
I noticed I have to press enter to search now. This doesn't feel natural, compared to the previous behavior that searches with a debounce
Screen.Recording.2023-05-25.at.10.41.50.AM.mov
-
Do you think we should change "Canceling action" and "Canceled action" to "Canceling" "Canceled" and use a grey dot?
-
The workspaces page shows "All users" workspaces by default, which is a change in behavior and makes it difficult for admins to reach their workspaces. What do you think about changing it to show
owner:me
by default, like the original? -
✅ Confirmed this works as expected with "template admin" and "user admin"
-
Searching on "page 2" with filters will not return to page 1 to find a result. Do you think we should add a test for this since it happened with our previous pagination behavior (then we fixed it?) I am concerned future refactors may not have this.
Screen.Recording.2023-05-25.at.10.55.04.AM.mov
-
When a final workspace is taken off page 2, the user is not taken to page 1 and it says there are no results
Screen.Recording.2023-05-25.at.10.57.19.AM.mov
-
Status: deleting doesn't seem to work.
Screen.Recording.2023-05-25.at.11.00.56.AM.mov
-
Can you check all of the statuses to make sure the filter works as expected? I am concerned there are more that do not filter, like "Deleting"
-
What do you think about adding "All statuses" and "All templates" as dropdown item? I don't think it is necessary but makes it a bit more clear you can unselect.
-
Status "stopping" shows me a failed workspace for some reason.
Screen.Recording.2023-05-25.at.11.04.12.AM.mov
I used coder scaletest create-workspaces --template=docker --concurreny=30 --count=100
to test this with a lot of workspaces and users.
Nice to have additional enhancements:
- Show # of results returned from a filter (e.g. I'd love to know how many workspaces are pending, or failed, or running)
- Somewhere to link to docs for advanced search syntax (e.g. has-agent)
- Add this design to the audit log & templates page. Audit log would be the most beneficial
@bpmct First of all, thanks for the QA and inputs ❤️
When having additional controls that can change the main search input, using a debounce was not working well. After clicking on a filter option, the user has to wait for the debounce delay which IMO makes it feels buggy. The possible solution for that also sounded kinda complex so I based the solution, on the user having to hit Enter to apply the search, on how Github does with their issues filter. Probably they faced similar issues.
I think this is a good one 👍
Adding a default filter makes things weird IMO since no filter in the URL should return an empty filter in the UI, it also makes it not easier to know when a user wants a clean filter or the default one. I also have been seeing a few users not wanting to use this filter for admins: #7586
Looks like it is an existing bug. If you try to use the query search by status on dev.coder.com, you will see that as well. I will open a ticket and see who can help me with that but IMO it should be another PR and not block the UI changes.
We can do that but I would wait for more feedback to do it. The reason is, having a button without an icon on the left breaks the design and we would have to think of a better way to display it. Extra: |
@bpmct I missed these:
This is a good one, let me think if there is an easy way to do this
This is a hard one.... but I will also think about it.
Totally, we can do this after this get merged. |
I found another bug: If I select a user in the filter, and after changing it in the search input, it does not update the dropdown filter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What is included:
Demo:
https://github.com/coder/coder/assets/3165839/1ecafeaa-4b1e-44ea-bc65-9335416b1617
PS: Tests are on the way but I want to validate this feature before writing them.