-
Notifications
You must be signed in to change notification settings - Fork 264
feat: squad list view #4710
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: squad list view #4710
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
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.
Minor comment
@@ -49,7 +49,8 @@ function FeedItemContainer( | |||
? `${trending} devs read it last hour` | |||
: undefined; | |||
const isFeedPreview = useFeedPreviewMode(); | |||
const showFlag = (!!pinnedAt || !!trending) && !isFeedPreview && description; | |||
const showFlag = |
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.
Why was this change needed?
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.
@rebelchris Because without the pinned flag isn't visible. I looked at freeform grid version of it now which is the following: (!!pinnedAt || !!trending) && !isFeedPreview}
. I'll change this one to be the same!
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.
Ah ok :)
@@ -214,29 +212,31 @@ const SquadPage = ({ handle, initialData }: SourcePageProps): ReactElement => { | |||
<SquadPageHeader | |||
squad={squad} | |||
members={squadMembers} | |||
shouldUseListMode={shouldUseListMode} | |||
shouldUseListMode={false} |
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.
We should either remove this or keep dynamic no?
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.
@rebelchris I checked with Tsahi. Waiting for feedback :)
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.
Just make sure to test both list and grid mode on all sizes (laptop, tablet, mobile).
Changes
Enables list view on squad page. While this was primarily done to update the mobile feed to be in line with the rest of the feeds, we decided to also allow list view on larger devices.
This also naturally fixes this and this bug
Mobile before:
ScreenRecording_07-17-2025.23-23-26_1.MP4
Mobile after:
ScreenRecording_07-18-2025.01-48-36_1.MP4
Desktop:
Screen.Recording.2025-07-18.at.01.51.19.mov
Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Jira ticket
MI-945
Preview domain
https://mi-945.preview.app.daily.dev