-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[projects] add missing search filter #5318
Conversation
Looking at this now! 👀 |
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.
Thanks for adding this @AlexTugarev! 🧙
Works good! Left two minor comments which could be ok as follow up issues. Your call!
</div> | ||
<Link to="./members" className="flex"><button className="ml-2 secondary">Invite Members</button></Link> | ||
<button className="ml-2" onClick={() => onNewProject()}>New Project</button> | ||
</div> | ||
<div className="mt-4 grid grid-cols-3 gap-4"> | ||
{projects.map(p => (<div key={`project-${p.id}`} className="h-48"> | ||
{projects.filter(filter).map(p => (<div key={`project-${p.id}`} className="h-48"> |
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.
suggestion: Could we hide the New Project card when there's no search result? Ideally, a minimal empty state like the one in #5022 with a link to create a new project could suffice. Sounds good also as a follow up issue. 🏓
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.
not showing the New Project
card on active search sounds great in general.
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.
fyi: Added follow up issue in #5343.
LGTM label has been added. Git tree hash: 23b97fa8d8affebcfe3d0f2d3cad3951a06359a0
|
1c70f87
to
5bdda33
Compare
/werft run 👍 started the job as gitpod-build-at-projects-search.2 @gtsiolis, I tackled half of the comments. Is it ok to merge for now? The label got removed automatically |
@AlexTugarev Absolutely. I think removing the card when search results are empty and adding an empty state can be part of another PR and maybe out of the scope of shipping this feature. 💭 🍊 🍊 🍊 🍊 note: Because of #5318 (comment), you'll also need an approval from someone from the engineering team but I could be wrong! |
/lgtm /approve |
/approve |
LGTM label has been added. Git tree hash: 6ca084efd0963aed25b16494450e9a79d2ef7c04
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexTugarev, gtsiolis Associated issue: #4972 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
this PR adds missing search filter for the projects page.
fixes #4972