-
Notifications
You must be signed in to change notification settings - Fork 4
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
ENG-4389 feat(portal,1ui): list card & carousel for featured lists #901
ENG-4389 feat(portal,1ui): list card & carousel for featured lists #901
Conversation
@@ -0,0 +1,22 @@ | |||
import * as React from 'react' | |||
|
|||
export interface ExploreListGridProps |
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.
Naming question on this @Vitalsine85 @0xjojikun -- do we want to call this something specific like Explore_
instead of a more generic name? Only reason I'm thinking about that is that this could be more generally useful for devs outside of the specific Explore usecase
Not a big deal by any means -- mainly something for us to think about as we create and name additional 1ui components
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 was actually thinking of just removing it from 1ui actually, since it is very specific to Portal. They already have ListGrid which serves the same purpose, we just needed another version for Explore because there were issues with how the grid was rendering from one page to another.
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.
That makes sense! That's likely the best approach if we don't really need it in 1ui and since it's Portal specific we can have it be there
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.
Read through everything and it looks good! I think a Loom or quick demo would be helpful with some context from the Figma
Nice work! 🔥
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 great!
92111c8
to
a01f377
Compare
Affected Packages
Apps
Packages
Tools
Overview
Screen Captures
I'll record a Loom tomorrow!
Declaration