-
Notifications
You must be signed in to change notification settings - Fork 979
feat: add user filter to templates page to filter by template author #19561
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.
I think this is overall fine. I'm just a little concerned that we're slapping band-aids on some critical code, rather than tightening it up. At the same time, I don't think it's reasonable to shove a bunch of changes into this PR
I'm okay with approving most of this, but I'm wondering if @BrunoQuaresma has any extra context for some small wins we can sneak in to make the code a little cleaner
|
||
interface TemplatesFilterProps { | ||
filter: ReturnType<typeof useFilter>; | ||
filter: UseFilterResult; |
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.
Much better. Thank you!
I'm good with the changes 👍 About the filter complexity, we def need to refactor it to make it simpler and easier to use. I think we talked about this sometime ago but no one was interested on moving this forward. |
@rafrdz Yeah, I don't think the new widths are quite right. I'm going through the code now to figure out what caused the change |
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.
I think we're pretty close and just need a few small changes
I'm going to approve to keep you unblocked, but if you need anyone to look over the stories, feel free to message me on Slack
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.
Whoops, actually approving
@BrunoQuaresma Yeah, sometime today or tomorrow, I can try to open up an issue about refactoring the setup. I'm going to be working on Backstage stuff pretty soon, but once that's wrapped up, I should have some bandwidth to work on updating all this |
Summary
In this pull request we're adding a user selector dropdown to the templates page that allows an admin to select a user. The selected user will be used in the
author:<username>
filter to filter the templates list by a template author.Closes: #19547
Changes
Admin View - Can view all users
Admin View - Using the user filter
templates-user-filter.mov
User view - Cannot view all users
Testing