Skip to content

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

Merged
merged 25 commits into from
May 30, 2023
Merged

Conversation

BrunoQuaresma
Copy link
Collaborator

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.

@BrunoQuaresma BrunoQuaresma requested review from bpmct and a team May 25, 2023 14:21
@BrunoQuaresma BrunoQuaresma self-assigned this May 25, 2023
@BrunoQuaresma BrunoQuaresma requested review from rodrimaia and removed request for a team May 25, 2023 14:21
Copy link
Member

@bpmct bpmct left a 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.

  1. 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
  2. Do you think we should change "Canceling action" and "Canceled action" to "Canceling" "Canceled" and use a grey dot?
    Screenshot 2023-05-25 at 10 40 33 AM

  3. 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?

  4. ✅ Confirmed this works as expected with "template admin" and "user admin"

  5. 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
  6. 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
  7. Status: deleting doesn't seem to work.

    Screen.Recording.2023-05-25.at.11.00.56.AM.mov
  8. 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"

  9. 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.

  10. 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

@BrunoQuaresma
Copy link
Collaborator Author

@bpmct First of all, thanks for the QA and inputs ❤️

  1. 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

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.

  1. Do you think we should change "Canceling action" and "Canceled action" to "Canceling" "Canceled" and use a grey dot?

I think this is a good one 👍

  1. 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?

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

  1. 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"

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.

  1. 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.

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:
About the pagination + filtering, good catch! I didn't test with that, I'm going to fix it rn. About the statuses, the problem should be the same and probably is BE stuff OR the FE is using the wrong label on the badges. I will take a look anyway.

@BrunoQuaresma
Copy link
Collaborator Author

@bpmct I missed these:

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

Show # of results returned from a filter (e.g. I'd love to know how many workspaces are pending, or failed, or running)

This is a good one, let me think if there is an easy way to do this

Somewhere to link to docs for advanced search syntax (e.g. has-agent)

This is a hard one.... but I will also think about it.

Add this design to the audit log & templates page. Audit log would be the most beneficial

Totally, we can do this after this get merged.

@BrunoQuaresma
Copy link
Collaborator Author

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.

@BrunoQuaresma BrunoQuaresma requested a review from bpmct May 25, 2023 21:23
Copy link
Contributor

@rodrimaia rodrimaia left a comment

Choose a reason for hiding this comment

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

LGTM

rodrimaia
rodrimaia approved these changes May 26, 2023
@BrunoQuaresma BrunoQuaresma merged commit 77b0ca0 into main May 30, 2023
@BrunoQuaresma BrunoQuaresma deleted the bq/workspace-filter branch May 30, 2023 17:52
@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve workspace filter Remove the WS filter owner:me when user is Owner
3 participants