-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add more integration icons to the website #134
Conversation
Signed-off-by: B-Step62 <[email protected]>
Signed-off-by: B-Step62 <[email protected]>
Signed-off-by: B-Step62 <[email protected]>
Preview for b00ab22
|
Signed-off-by: B-Step62 <[email protected]>
Signed-off-by: B-Step62 <[email protected]>
Signed-off-by: B-Step62 <[email protected]>
const ExpandableGrid = ({ items, defaultVisibleCount, renderItem }) => { | ||
const [isExpanded, setIsExpanded] = useState(false); | ||
|
||
const visibleItems = isExpanded ? items : items.slice(0, defaultVisibleCount); |
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.
nits: we could use useMemo here, but it is probably not necessary since this should be a lightweight app and this component won't be rendered for many times.
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.
yea lemme keep it simple here for now
const visibleItems = isExpanded ? items : items.slice(0, defaultVisibleCount); | ||
|
||
return ( | ||
<div> |
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.
do we need this top level div?
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 really - can replace with <></>
🙂
Looks good, will defer to others who are more familiar with the repo. |
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!
Signed-off-by: B-Step62 <[email protected]>
We are only showing 12 libraries on website but we have much more integrations. Adding most of them with expand/collapse.