-
Notifications
You must be signed in to change notification settings - Fork 82
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
feat: add search for collections #3095
Conversation
meelrossi
commented
May 3, 2024
•
edited
Loading
edited
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Pull Request Test Coverage Report for Build 8944820620Details
💛 - Coveralls |
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.
Great job! 👍
There seem to be an issue with the search bar and the right buttons. The search bar seems to be stretching too much:
And, I'm not sure if it's related to this change, but when there's no collection available, there's a big gap between the search bar and the nav bar. It seems that as there are no tabs, the space between the components remains:
Would you mind taking a look at it?
@@ -31,13 +34,15 @@ import CollectionRow from './CollectionRow' | |||
import { Props, TABS } from './CollectionsPage.types' | |||
import './CollectionsPage.css' | |||
|
|||
let timeout: NodeJS.Timeout |
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.
As the CollectionsPage
component lives in the web environment, we can ask for the return type of the setTimeout instead of typing it as the NodeJS.Timeout
one:
let timeout: NodeJS.Timeout | |
let timeout: ReturnType<typeof setTimeout> |
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.
Would you mind defining the timeout
variable as being part of the CollectionsPage
component instance so it's not globally defined?
aa972bf
to
28957c0
Compare
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 amazing, great work 🚀