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.

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

Reader: Filter main feed based on sidebar selection #97123

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 8 additions & 29 deletions client/reader/sidebar/index.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { isEnabled } from '@automattic/calypso-config';
import page from '@automattic/calypso-router';
import { hasTranslation } from '@wordpress/i18n';
import closest from 'component-closest';
import i18n, { localize } from 'i18n-calypso';
import { defer, startsWith } from 'lodash';
Expand All @@ -20,7 +19,6 @@ import SidebarSeparator from 'calypso/layout/sidebar/separator';
import ReaderA8cConversationsIcon from 'calypso/reader/components/icons/a8c-conversations-icon';
import ReaderConversationsIcon from 'calypso/reader/components/icons/conversations-icon';
import ReaderDiscoverIcon from 'calypso/reader/components/icons/discover-icon';
import ReaderFollowingIcon from 'calypso/reader/components/icons/following-icon';
import ReaderLikesIcon from 'calypso/reader/components/icons/likes-icon';
import ReaderManageSubscriptionsIcon from 'calypso/reader/components/icons/manage-subscriptions-icon';
import ReaderNotificationsIcon from 'calypso/reader/components/icons/notifications-icon';
Expand Down Expand Up @@ -117,13 +115,6 @@ export class ReaderSidebar extends Component {
} );
};

handleReaderSidebarFollowedSitesClicked = ( event, path ) => {
recordAction( 'clicked_reader_sidebar_followed_sites' );
recordGaEvent( 'Clicked Reader Sidebar Followed Sites' );
this.props.recordReaderTracksEvent( 'calypso_reader_sidebar_followed_sites_clicked' );
this.handleGlobalSidebarMenuItemClick( path );
};

handleReaderSidebarConversationsClicked = ( event, path ) => {
recordAction( 'clicked_reader_sidebar_conversations' );
recordGaEvent( 'Clicked Reader Sidebar Conversations' );
Expand Down Expand Up @@ -174,8 +165,8 @@ export class ReaderSidebar extends Component {
};

renderSidebarMenu() {
const { path, translate, teams, locale } = this.props;
const recentLabelTranslationReady = hasTranslation( 'Recent' ) || locale.startsWith( 'en' );
const { path, translate, teams } = this.props;

return (
<SidebarMenu>
<QueryReaderLists />
Expand All @@ -194,25 +185,13 @@ export class ReaderSidebar extends Component {

<SidebarSeparator />

{ isEnabled( 'reader/recent-feed-overhaul' ) ? (
<li className="sidebar-streams__following">
<ReaderSidebarRecent
onClick={ this.props.toggleFollowingVisibility }
isOpen={ this.props.isFollowingOpen }
path={ path }
/>
</li>
) : (
<SidebarItem
className={ ReaderSidebarHelper.itemLinkClass( '/read', path, {
'sidebar-streams__following': true,
} ) }
label={ recentLabelTranslationReady ? translate( 'Recent' ) : translate( 'Following' ) }
onNavigate={ this.handleReaderSidebarFollowedSitesClicked }
customIcon={ <ReaderFollowingIcon viewBox="-3 0 24 24" /> }
link="/read"
<li className="sidebar-streams__following">
mehmoodak marked this conversation as resolved.
Show resolved Hide resolved
<ReaderSidebarRecent
onClick={ this.props.toggleFollowingVisibility }
isOpen={ this.props.isFollowingOpen }
path={ path }
/>
) }
</li>

<SidebarItem
className={ ReaderSidebarHelper.itemLinkClass( '/discover', path, {
Expand Down
14 changes: 14 additions & 0 deletions client/reader/sidebar/reader-sidebar-recent/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { useDispatch, useSelector } from 'react-redux';
import ExpandableSidebarMenu from 'calypso/layout/sidebar/expandable';
import Favicon from 'calypso/reader/components/favicon';
import ReaderFollowingIcon from 'calypso/reader/components/icons/following-icon';
import { recordAction, recordGaEvent } from 'calypso/reader/stats';
import { useRecordReaderTracksEvent } from 'calypso/state/reader/analytics/useRecordReaderTracksEvent';
import getReaderFollowedSites from 'calypso/state/reader/follows/selectors/get-reader-followed-sites';
import { selectSidebarRecentSite } from 'calypso/state/reader-ui/sidebar/actions';
import { AppState } from 'calypso/types';
Expand Down Expand Up @@ -50,6 +52,7 @@ const ReaderSidebarRecent = ( {
const selectedSiteFeedId = useSelector< AppState, number >(
( state ) => state.readerUi.sidebar.selectedRecentSite
);
const recordReaderTracksEvent = useRecordReaderTracksEvent();
const dispatch = useDispatch();

let sitesToShow = showAllSites ? sites : sites.slice( 0, SITE_DISPLAY_CUTOFF );
Expand All @@ -75,6 +78,17 @@ const ReaderSidebarRecent = ( {
if ( ! RECENT_PATH_REGEX.test( path ) ) {
page( '/read' );
}

// Analytics.
if ( feedId ) {
recordAction( 'clicked_reader_sidebar_followed_single_site' );
recordGaEvent( 'Clicked Reader Sidebar Followed Single Site' );
recordReaderTracksEvent( 'calypso_reader_sidebar_followed_single_site_clicked' );
mehmoodak marked this conversation as resolved.
Show resolved Hide resolved
} else {
recordAction( 'clicked_reader_sidebar_followed_sites' );
recordGaEvent( 'Clicked Reader Sidebar Followed Sites' );
recordReaderTracksEvent( 'calypso_reader_sidebar_followed_sites_clicked' );
}
};

return (
Expand Down
62 changes: 48 additions & 14 deletions client/reader/stream/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { getReaderOrganizations } from 'calypso/state/reader/organizations/selec
import { getPostByKey } from 'calypso/state/reader/posts/selectors';
import { getBlockedSites } from 'calypso/state/reader/site-blocks/selectors';
import {
clearStream,
requestPage,
selectItem,
selectNextItem,
Expand All @@ -42,6 +43,7 @@ import {
} from 'calypso/state/reader/streams/selectors';
import { viewStream } from 'calypso/state/reader-ui/actions';
import { resetCardExpansions } from 'calypso/state/reader-ui/card-expansions/actions';
import { getSelectedFeedId } from 'calypso/state/reader-ui/sidebar/selectors';
import getCurrentLocaleSlug from 'calypso/state/selectors/get-current-locale-slug';
import getPrimarySiteId from 'calypso/state/selectors/get-primary-site-id';
import isNotificationsOpen from 'calypso/state/selectors/is-notifications-open';
Expand Down Expand Up @@ -126,8 +128,13 @@ class ReaderStream extends Component {
this.setState( { selectedTab: 'sites' } );
};

componentDidUpdate( { selectedPostKey, streamKey } ) {
if ( streamKey !== this.props.streamKey ) {
componentDidUpdate( { selectedPostKey, streamKey, selectedFeedId } ) {
// Fetch new page if selected feed or stream is changed.
if ( selectedFeedId !== this.props.selectedFeedId ) {
this.scrollFeedListToTop();
this.props.clearStream( { streamKey } );
this.fetchNextPage( {}, { ...this.props, stream: null } ); // Stream as null to start fresh pagination.
} else if ( streamKey !== this.props.streamKey ) {
this.props.resetCardExpansions();
this.props.viewStream( streamKey, window.location.pathname );
this.fetchNextPage( {} );
Expand All @@ -147,6 +154,7 @@ class ReaderStream extends Component {
if ( this.props.shouldRequestRecs ) {
this.props.requestPage( {
streamKey: this.props.recsStreamKey,
feedId: this.props.selectedFeedId,
pageHandle: this.props.recsStream.pageHandle,
localeSlug: this.props.localeSlug,
} );
Expand Down Expand Up @@ -447,8 +455,13 @@ class ReaderStream extends Component {
};

poll = () => {
const { streamKey, localeSlug } = this.props;
this.props.requestPage( { streamKey, isPoll: true, localeSlug: localeSlug } );
const { streamKey, localeSlug, selectedFeedId } = this.props;
this.props.requestPage( {
streamKey,
feedId: selectedFeedId,
isPoll: true,
localeSlug: localeSlug,
} );
};

getPageHandle = ( pageHandle, startDate ) => {
Expand All @@ -461,28 +474,30 @@ class ReaderStream extends Component {
};

fetchNextPage = ( options, props = this.props ) => {
const { streamKey, stream, startDate, localeSlug } = props;
const { streamKey, stream, startDate, localeSlug, selectedFeedId } = props;
if ( options.triggeredByScroll ) {
const pageId = pagesByKey.get( streamKey ) || 0;
pagesByKey.set( streamKey, pageId + 1 );

props.trackScrollPage( pageId );
}
const pageHandle = this.getPageHandle( stream.pageHandle, startDate );
props.requestPage( { streamKey, pageHandle, localeSlug } );
const pageHandle = stream ? this.getPageHandle( stream.pageHandle, startDate ) : null;
props.requestPage( { feedId: selectedFeedId, streamKey, pageHandle, localeSlug } );
};

showUpdates = () => {
const { streamKey } = this.props;
this.props.onUpdatesShown();
this.props.showUpdates( { streamKey } );
// @todo: do we need to shuffle?
// if ( this.props.recommendationsStore ) {
// shufflePosts( this.props.recommendationsStore.id );
// }
if ( this.listRef.current ) {
this.listRef.current.scrollToTop();
this.scrollFeedListToTop();
};

scrollFeedListToTop = () => {
if ( ! this.listRef.current ) {
return;
}

this.listRef.current.scrollToTop();
};

renderLoadingPlaceholders = () => {
Expand Down Expand Up @@ -726,8 +741,24 @@ class ReaderStream extends Component {
}
}

/**
* Returns a modified stream key if necessary else returns the original stream key.
* @returns {string} Stream key.
*/
function getStreamKey( state, streamKey ) {
// For "following" stream, use a unique streamKey if a feed is selected. This prevent feed overwrites when rapid selections are made.
const selectedFeedId = getSelectedFeedId( state );
const isFollowingFiltered = streamKey === 'following' && selectedFeedId;
if ( isFollowingFiltered ) {
return `following:feed-${ selectedFeedId }`;
}

return streamKey;
}

export default connect(
( state, { streamKey, recsStreamKey } ) => {
( state, { streamKey: tempStreamKey, recsStreamKey } ) => {
const streamKey = getStreamKey( state, tempStreamKey );
const stream = getStream( state, streamKey );
const selectedPost = getPostByKey( state, stream.selected );

Expand All @@ -744,7 +775,9 @@ export default connect(
} ),
notificationsOpen: isNotificationsOpen( state ),
stream,
streamKey,
recsStream: getStream( state, recsStreamKey ),
selectedFeedId: getSelectedFeedId( state ),
selectedPostKey: stream.selected,
selectedPost,
lastPage: stream.lastPage,
Expand All @@ -757,6 +790,7 @@ export default connect(
};
},
{
clearStream,
resetCardExpansions,
likePost,
unlikePost,
Expand Down
7 changes: 4 additions & 3 deletions client/state/data-layer/wpcom/read/streams/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ const streamApis = {
*/
export function requestPage( action ) {
const {
payload: { streamKey, streamType, pageHandle, isPoll, gap, localeSlug, page, perPage },
payload: { streamKey, streamType, feedId, pageHandle, isPoll, gap, localeSlug, page, perPage },
} = action;
const api = streamApis[ streamType ];

Expand Down Expand Up @@ -385,15 +385,16 @@ export function requestPage( action ) {
// There is a race condition in switchLocale when retrieving the language file
// The stream request can occur before the language file is loaded, so we need a way to explicitly set the lang in the request
const lang = localeSlug || i18n.getLocaleSlug();
const commonQueryParams = { ...algorithm, feed_id: feedId };

return http( {
method: 'GET',
path: path( { ...action.payload } ),
apiVersion,
apiNamespace: api.apiNamespace ?? null,
query: isPoll
? pollQuery( [], { ...algorithm } )
: query( { ...pageHandle, ...algorithm, number, lang, page }, action.payload ),
? pollQuery( [], commonQueryParams )
: query( { ...commonQueryParams, ...pageHandle, number, lang, page }, action.payload ),
onSuccess: action,
onFailure: action,
} );
Expand Down
4 changes: 2 additions & 2 deletions client/state/data-layer/wpcom/read/streams/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ describe( 'streams', () => {

it( 'should set proper params for subsequent fetches', () => {
const pageHandle = { after: 'the robots attack' };
const secondPage = { ...action, payload: { ...action.payload, pageHandle } };
const secondPage = { ...action, payload: { ...action.payload, pageHandle, feedId: 1234 } };
const httpAction = requestPage( secondPage );

expect( httpAction ).toEqual(
http( {
method: 'GET',
path: '/read/following',
apiVersion: '1.2',
query: { ...query, number: PER_FETCH, after: 'the robots attack' },
query: { ...query, feed_id: 1234, number: PER_FETCH, after: 'the robots attack' },
onSuccess: secondPage,
onFailure: secondPage,
} )
Expand Down
1 change: 0 additions & 1 deletion client/state/reader-ui/sidebar/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export function toggleReaderSidebarFollowing() {
* @returns The action object to dispatch.
*/
export function selectSidebarRecentSite( { feedId } ) {
// @holdercp TODO: Determine if tracking is needed here
return {
type: READER_SIDEBAR_SELECT_RECENT_SITE,
feedId,
Expand Down
42 changes: 0 additions & 42 deletions client/state/reader-ui/sidebar/selectors.js

This file was deleted.

41 changes: 41 additions & 0 deletions client/state/reader-ui/sidebar/selectors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import 'calypso/state/reader-ui/init';
import { AppState } from 'calypso/types';

/**
* Whether or not a specific reader organization sidebar item is open.
*/
export function isOrganizationOpen( state: AppState, organizationId: number ): boolean {
const openOrganizations = state.readerUi.sidebar.openOrganizations;
if ( openOrganizations.indexOf( organizationId ) > -1 ) {
return true;
}
return false;
}

/**
* Whether or not following reader sidebar item is open.
*/
export function isFollowingOpen( state: AppState ): boolean {
return state.readerUi.sidebar.isFollowingOpen;
}

/**
* Whether or not lists reader sidebar item is open.
*/
export function isListsOpen( state: AppState ): boolean {
return state.readerUi.sidebar.isListsOpen;
}

/**
* Whether or not tags reader sidebar item is open.
*/
export function isTagsOpen( state: AppState ): boolean {
return state.readerUi.sidebar.isTagsOpen;
}

/**
* Get the selected feed ID from the reader sidebar.
*/
export function getSelectedFeedId( state: AppState ): number | null {
return state.readerUi.sidebar.selectedRecentSite;
}
mehmoodak marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading