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

Reader: Update action button link of an empty list of a user profile #98706

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
+57 −47
Open

Reader: Update action button link of an empty list of a user profile #98706

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6140385
💫 UPDATE: migrate ListEmptyContent to function component with TS support
mehmoodak Jan 21, 2025
6a41d2c
💫 UPDATE: action link on empty list of a user profile
mehmoodak Jan 21, 2025
ac8eff3
🐛 FIX: possible XSS
mehmoodak Jan 21, 2025
f91cb69
🐛 FIX: url check via RegExp
mehmoodak Jan 22, 2025
376a54f
🐛 FIX: fire track events for both cases
mehmoodak Jan 22, 2025
1249bb2
🐛 FIX: WSOD issue due to url encoding
mehmoodak Jan 22, 2025
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
46 changes: 0 additions & 46 deletions client/reader/list-stream/empty.jsx
View file
Open in desktop

This file was deleted.

56 changes: 56 additions & 0 deletions client/reader/list-stream/empty.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { useTranslate } from 'i18n-calypso';
import EmptyContent from 'calypso/components/empty-content';
import { recordAction, recordGaEvent } from 'calypso/reader/stats';
import { useDispatch } from 'calypso/state';
import { recordReaderTracksEvent } from 'calypso/state/reader/analytics/actions';

export default function ListEmptyContent(): JSX.Element {
const dispatch = useDispatch();
const translate = useTranslate();
const queryParams = new URLSearchParams( location.search );
const lastPageLink = queryParams.get( 'last_page' ) ?? '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we pull this out of the URL and avoid keeping extra query params in the URL? We could probably look for the value on mount, remove it from the url, and use that initial info for the text/href.

Copy link
Member Author

@mehmoodak mehmoodak Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

remove it from the url

I avoided it because it will create a new entry in the browser history.


function onClickActionBtn(): void {
if ( lastPageLink ) {
return;
}

recordAction( 'clicked_following_on_empty' );
recordGaEvent( 'Clicked Following on EmptyContent' );
dispatch( recordReaderTracksEvent( 'calypso_reader_following_on_empty_list_stream_clicked' ) );
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like we are avoiding firing the events around clicking for the following feed. I wonder if there is any value to these types of events, and if we should consider sending separate ones in the case of profile (or alternatively removing them all if not valuable).

Copy link
Member Author

Choose a reason for hiding this comment

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

I also thought of removing this. Let me know if we need to remove this.

For now I am just removing the if condition as firing the events for both cases is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 376a54f

}

function lastPageIsUserProfileLists(): boolean {
return /^\/read\/users\/[a-z0-9]+\/lists$/.test( lastPageLink );
}

function getActionBtnLink(): string {
return lastPageIsUserProfileLists() ? encodeURIComponent( lastPageLink ) : '/read';
}

function getActionBtnText(): string {
return lastPageIsUserProfileLists()
? translate( 'Back to User Profile' )
: translate( 'Back to Following' );
}

const action = (
<a
className="empty-content__action button is-primary"
onClick={ onClickActionBtn }
href={ getActionBtnLink() }
Dismissed Show dismissed Hide dismissed
Fixed Show fixed Hide fixed
>
{ getActionBtnText() }
</a>
);

return (
<EmptyContent
title={ translate( 'No recent posts' ) }
line={ translate( 'The sites in this list have not posted anything recently.' ) }
action={ action }
illustration="/calypso/images/illustrations/illustration-empty-results.svg"
illustrationWidth={ 400 }
/>
);
}
5 changes: 4 additions & 1 deletion client/reader/user-profile/views/lists.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import EmptyContent from 'calypso/components/empty-content';
import { UserData } from 'calypso/lib/user/user';
import { List } from 'calypso/reader/list-manage/types';
import UserProfileHeader from 'calypso/reader/user-profile/components/user-profile-header';
import { getUserProfileUrl } from 'calypso/reader/user-profile/user-profile.utils';
import { requestUserLists } from 'calypso/state/reader/lists/actions';

interface AppState {
Expand Down Expand Up @@ -54,14 +55,16 @@ const UserLists = ( { user, requestUserLists, lists, isLoading }: UserListsProps
);
}

const userProfileListsUrl = `${ getUserProfileUrl( user.ID ) }/lists`;

return (
<div className="user-profile__lists">
<UserProfileHeader user={ user } />
<div className="user-profile__lists-body">
{ lists.map( ( list: List ) => (
<a
className="user-profile__lists-body-link"
href={ `/read/list/${ list.owner }/${ list.slug }` }
href={ `/read/list/${ list.owner }/${ list.slug }?last_page=${ userProfileListsUrl }` }
key={ list.ID }
>
<div className="card reader-post-card is-compact is-clickable">
Expand Down
Loading