chore: cleanup old list mode #3179
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| @@ -19,7 +29,7 @@ export function UserShortInfoPlaceholder({ | |||
| .fill(0) | |||
| .map((_, i) => ( | |||
| // eslint-disable-next-line react/no-array-index-key | |||
| <PlaceholderList key={i} /> | |||
| <Placeholder key={i} /> | |||
There was a problem hiding this comment.
This was using the card list placeholder which didn't even match, so made a new matching one 🙏
There was a problem hiding this comment.
Other possible cleanups:
- forceCardMode as well doesn't apply anymore - I think it was a good think if we plan to refactor after we can rethink if we add this again.
- In Feed & FeedContainer there is a useList and insane mode that we send to feedItemComponent that we don't need anymore
- ActionButtons have an insaneMode prop that we never send to it
- We send an insane mode to cards not on V1 which I don't think is used anymore
- It would be good if we would rename the V1 cards to something with List.
- There's something about showImage={!insaneMode} that we send to post cards that's not relevant anymore.
Practically needs a bit of search on insaneMode usage - I can help with doing the rest of the changes Chris if u want. Also in general maybe for future the whole thing of feed layout and the 2 types of cards would need a refactor as it's become a bit messy.
| @@ -10,16 +10,6 @@ import { SourceMember } from './sources'; | |||
| import { graphqlUrl } from '../lib/config'; | |||
There was a problem hiding this comment.
Nice! Thank you for adding this cleanup ❤️
Just remembered: do we have the ticket to clean up the rest of legacy weekly rank? CSS files, etc. @nensidosari
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't know why, but I think I have a personal vendetta against the legacy weekly goals, probably due to the problems it gave us back then 😂
…p-feed_list_layout # Conflicts: # packages/shared/src/components/Feed.tsx
| const isList = | ||
| (isHorizontal || isListModeV1) && !shouldUseListFeedLayout | ||
| (isHorizontal || isListMode) && !shouldUseListFeedLayout | ||
| ? false | ||
| : (insaneMode && numCards > 1) || shouldUseListFeedLayout; | ||
| : (isListMode && numCards > 1) || shouldUseListFeedLayout; |
There was a problem hiding this comment.
I got the feeling this can now also be simplified as it contained logic for old list mode - but will need to check.
There was a problem hiding this comment.
Alright will leave it for this task as It's already a bit much.
Do think we can as we have also stuff like this: (which I think just is the same thing twice 😅)
gapClass({
isList,
isFeedLayoutList: shouldUseListFeedLayout,
Changes
Describe what this PR does
Events
Did you introduce any new tracking events?
Don't forget to update the Analytics Taxonomy sheet
Log the new/changed events below:
Please make sure existing components are not breaking/affected by this PR
Manual Testing
On those affected packages:
Did you test the modified components media queries?
Did you test on actual mobile devices?
AS-360 #done
Preview domain
https://as-360-cleanup-feed-list-layout.preview.app.daily.dev