Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Hide empty content in favor of Reader onboarding. #96988

Merged
merged 7 commits into from
Dec 3, 2024
+108 −107
Merged

Hide empty content in favor of Reader onboarding. #96988

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c714e55
Hide empty content in favor of Reader onboarding.
allilevine Dec 2, 2024
864bcce
Refactor so that both views show the same ReaderOnboarding component.
allilevine Dec 2, 2024
7c03e46
Add back welcome message.
allilevine Dec 2, 2024
8d0bd58
Remove hasSubscriptions prop, not needed.
allilevine Dec 2, 2024
5d7f75d
Show Reader Onboarding if the user is only subscribed to their own si…
allilevine Dec 3, 2024
559555c
Prevent the flash of onboarding before the streams load.
allilevine Dec 3, 2024
4ecd617
Remove redundant isLoading check.
allilevine Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions client/reader/following/main.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import config from '@automattic/calypso-config';
import { SubscriptionManager } from '@automattic/data-stores';
import clsx from 'clsx';
import { translate } from 'i18n-calypso';
import { useMemo } from 'react';
import AsyncLoad from 'calypso/components/async-load';
import BloganuaryHeader from 'calypso/components/bloganuary-header';
import NavigationHeader from 'calypso/components/navigation-header';
Expand All @@ -16,9 +18,54 @@ import './style.scss';

function FollowingStream( { ...props } ) {
const { currentView } = useFollowingView();
const { data: subscriptionsCount, isLoading: isLoadingCount } =
SubscriptionManager.useSubscriptionsCountQuery();
const { data: siteSubscriptions, isLoading: isLoadingSiteSubscriptions } =
SubscriptionManager.useSiteSubscriptionsQuery();

const isLoading = isLoadingCount || isLoadingSiteSubscriptions;

const hasNonSelfSubscriptions = useMemo( () => {
if ( isLoading ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check in this function? I think having it within the hasNonSelfSubscriptions function muddies the intent of the function, meaning the value really represents hasNonSelfSubscriptionsAndNotLoadingSubscriptions 😂.

Can we move the loading check outside of this function? It looks like we're checking isLoading again on line 50 so maybe this check is redundant anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it's redundant. I went ahead and removed it. 👍

return true;
}

if ( ! subscriptionsCount?.blogs || subscriptionsCount.blogs === 0 ) {
return false;
}

// If we have site subscriptions data, filter out self-owned blogs.
if ( siteSubscriptions?.subscriptions ) {
const nonSelfSubscriptions = siteSubscriptions.subscriptions.filter(
( sub ) => ! sub.is_owner
);
return nonSelfSubscriptions.length > 0;
}

return subscriptionsCount.blogs > 0;
}, [ subscriptionsCount?.blogs, siteSubscriptions, isLoading ] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does using the optional chaining here impact useMemo? What if it resolves to undefined? I would suspect it would be fine and would work like we expect and re-run the function if that first item changes from undefined to an array or object.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah if subscriptionsCount is undefined, it would just re-run. I updated to explicitly check the object.


const viewToggle = config.isEnabled( 'reader/recent-feed-overhaul' ) ? <ViewToggle /> : null;

if ( ! isLoading && ! hasNonSelfSubscriptions ) {
return (
<div className="following-stream--no-subscriptions">
<NavigationHeader title={ translate( 'Recent' ) } />
<p>
{ translate(
'{{strong}}Welcome!{{/strong}} Follow your favorite sites and their latest posts will appear here. Read, like, and comment in a distraction-free environment. Get started by selecting your interests below:',
{
components: {
strong: <strong />,
},
}
) }
</p>
<ReaderOnboarding forceShow />
</div>
);
}

return (
<>
{ currentView === 'recent' && config.isEnabled( 'reader/recent-feed-overhaul' ) ? (
Expand Down
12 changes: 12 additions & 0 deletions client/reader/following/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import "@wordpress/base-styles/breakpoints";

.is-section-reader .navigation-header.following-stream-header {
margin: 0 auto;
max-width: 600px;
Expand Down Expand Up @@ -191,3 +193,13 @@
}
}
}

.following-stream--no-subscriptions {
max-width: $break-medium;
margin: 0 auto;
padding: 16px 24px;

.navigation-header {
padding-bottom: 0;
}
}
133 changes: 53 additions & 80 deletions client/reader/recent/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { SubscriptionManager } from '@automattic/data-stores';
import { WIDE_BREAKPOINT } from '@automattic/viewport';
import { useBreakpoint } from '@automattic/viewport-react';
import { DataViews, filterSortAndPaginate, View } from '@wordpress/dataviews';
Expand All @@ -14,7 +13,6 @@ import NavigationHeader from 'calypso/components/navigation-header';
import { getPostByKey } from 'calypso/state/reader/posts/selectors';
import { requestPaginatedStream } from 'calypso/state/reader/streams/actions';
import { viewStream } from 'calypso/state/reader-ui/actions';
import ReaderOnboarding from '../onboarding';
import EngagementBar from './engagement-bar';
import RecentPostField from './recent-post-field';
import RecentPostSkeleton from './recent-post-skeleton';
Expand Down Expand Up @@ -159,93 +157,68 @@ const Recent = ( { viewToggle }: RecentProps ) => {
setIsLoading( data?.isRequesting );
}, [ data?.isRequesting ] );

const { data: subscriptionsCount } = SubscriptionManager.useSubscriptionsCountQuery();
const hasSubscriptions = subscriptionsCount?.blogs && subscriptionsCount.blogs > 0;

return (
<div className="recent-feed">
<div
className={ `recent-feed__list-column ${
selectedItem && hasSubscriptions ? 'has-overlay' : ''
} ${ ! hasSubscriptions ? 'recent-feed--no-subscriptions' : '' }` }
>
<div className={ `recent-feed__list-column ${ selectedItem ? 'has-overlay' : '' }` }>
<div className="recent-feed__list-column-header">
<NavigationHeader title={ translate( 'Recent' ) }>{ viewToggle }</NavigationHeader>
</div>
<div className="recent-feed__list-column-content">
{ ! hasSubscriptions ? (
<>
<p>
{ translate(
'{{strong}}Welcome!{{/strong}} Follow your favorite sites and their latest posts will appear here. Read, like, and comment in a distraction-free environment. Get started by selecting your interests below:',
{
components: {
strong: <strong />,
},
}
) }
</p>
<ReaderOnboarding forceShow />
</>
) : (
<DataViews
getItemId={ ( item: ReaderPost, index = 0 ) =>
item.postId?.toString() ?? `item-${ index }`
}
view={ view as View }
fields={ fields }
data={ shownData }
onChangeView={ ( newView: View ) =>
setView( {
type: newView.type,
fields: newView.fields ?? [],
layout: view.layout,
perPage: newView.perPage,
page: newView.page,
search: newView.search,
} )
}
paginationInfo={ view.search === '' ? defaultPaginationInfo : paginationInfo }
defaultLayouts={ { list: {} } }
isLoading={ isLoading }
selection={ selectedItem ? [ selectedItem.postId?.toString() ] : [] }
onChangeSelection={ ( newSelection: string[] ) => {
const selectedPost = data?.items?.find(
( item: ReaderPost ) => item.postId?.toString() === newSelection[ 0 ]
);
setSelectedItem( selectedPost || null );
} }
/>
) }
<DataViews
getItemId={ ( item: ReaderPost, index = 0 ) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 I think pulling all the subscription logic up into main makes this much easier to reason about.

item.postId?.toString() ?? `item-${ index }`
}
view={ view as View }
fields={ fields }
data={ shownData }
onChangeView={ ( newView: View ) =>
setView( {
type: newView.type,
fields: newView.fields ?? [],
layout: view.layout,
perPage: newView.perPage,
page: newView.page,
search: newView.search,
} )
}
paginationInfo={ view.search === '' ? defaultPaginationInfo : paginationInfo }
defaultLayouts={ { list: {} } }
isLoading={ isLoading }
selection={ selectedItem ? [ selectedItem.postId?.toString() ] : [] }
onChangeSelection={ ( newSelection: string[] ) => {
const selectedPost = data?.items?.find(
( item: ReaderPost ) => item.postId?.toString() === newSelection[ 0 ]
);
setSelectedItem( selectedPost || null );
} }
/>
</div>
</div>
{ hasSubscriptions ? (
<div className={ `recent-feed__post-column ${ selectedItem ? 'overlay' : '' }` }>
{ ! ( selectedItem && getPostFromItem( selectedItem ) ) && isLoading && (
<RecentPostSkeleton />
) }
{ ! isLoading && data?.items.length === 0 && (
<EmptyContent
title={ translate( 'Nothing Posted Yet' ) }
line={ translate( 'This feed is currently empty.' ) }
illustration="/calypso/images/illustrations/illustration-empty-results.svg"
illustrationWidth={ 400 }
<div className={ `recent-feed__post-column ${ selectedItem ? 'overlay' : '' }` }>
{ ! ( selectedItem && getPostFromItem( selectedItem ) ) && isLoading && (
<RecentPostSkeleton />
) }
{ ! isLoading && data?.items.length === 0 && (
<EmptyContent
title={ translate( 'Nothing Posted Yet' ) }
line={ translate( 'This feed is currently empty.' ) }
illustration="/calypso/images/illustrations/illustration-empty-results.svg"
illustrationWidth={ 400 }
/>
) }
{ data?.items.length > 0 && selectedItem && getPostFromItem( selectedItem ) && (
<>
<AsyncLoad
require="calypso/blocks/reader-full-post"
feedId={ selectedItem.feedId }
postId={ selectedItem.postId }
onClose={ () => setSelectedItem( null ) }
layout="recent"
/>
) }
{ data?.items.length > 0 && selectedItem && getPostFromItem( selectedItem ) && (
<>
<AsyncLoad
require="calypso/blocks/reader-full-post"
feedId={ selectedItem.feedId }
postId={ selectedItem.postId }
onClose={ () => setSelectedItem( null ) }
layout="recent"
/>
<EngagementBar feedId={ selectedItem?.feedId } postId={ selectedItem?.postId } />
</>
) }
</div>
) : null }
<EngagementBar feedId={ selectedItem?.feedId } postId={ selectedItem?.postId } />
</>
) }
</div>
</div>
);
};
Expand Down
27 changes: 0 additions & 27 deletions client/reader/recent/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,33 +148,6 @@
padding: $recent-feed-spacing;
}
}

&.recent-feed--no-subscriptions {
max-width: none;

.recent-feed__list-column-header {
max-width: none;
margin: 0;
padding: 16px 24px;
border-bottom: 1px solid var( --studio-gray-0 );

header.navigation-header {
max-width: $break-medium;
margin: 0 auto;
padding: 0;
}
}

.recent-feed__list-column-content {
max-width: $break-medium;
margin: 0 auto;
padding: 24px 24px 0;
}

@media ( min-width: $break-wide ) {
max-width: none;
}
}
}

&__post-column {
Expand Down
Loading