app-project: Redesign how your stats are displayed on the classify page #6472

goplayoutside3 · 2024-11-15T23:09:19Z

Writing unit tests
Passing a mutate function to the classifier so stats are incremented on each submit
Do a design review with Sean
Ready for review!

Package

app-project

Linked Issue and/or Talk Post

Closes: #6400
Closes: #5964
Closes: #5134
Closes: #853

Describe your changes

This PR is a follow-up to ADR 61: Displaying Stats in UTC Date Ranges #6442 and part of a larger effort to display user stats consistently across the website.
I deleted all legacy YourStats code from the MobX store in favor of a new component that uses ERAS and SWR. The new component is called YourProjectStats to avoid a messy diff, but I can give it the name YourStats if preferred.
I deleted the FinishedForTheDay component at Sean's design request.
I created a RequireUser presentational component and deleted the legacy withRequireUser component. The legacy component was only used in the legacy YourStats component, and I prefer to have a simple "you should sign in" UI component to import into areas we want to hide when a user isn't signed in. In the future, I plan to use it in the Recents pages, etc.
Merging this PR means all of VisX can be removed from app-project. That will be done in a separate PR though in order to leave yarn.lock untouched here.

How to Review

I recommend reviewing this code as if I'm building something brand new to app-project rather than comparing to the old YourStats code.
See further notes about unit tests, translations, etc below.
The SWR hook pays attention to project ID (and uses it to cache requests), so any project's classify page you visit should show your stats specific to that project (active projects sheet for ref).
The stats per project should also match your personal /stats page. For instance, the project number of i-fancy-cats is 335, and the ERAS query for stats on the project's classify page and your personal bar chart at https://www.zooniverse.org/users/goplayoutside3/stats?env=staging&project_id=335 are the same.
Your stats should increment every time you make a classification. Try making several classifications, refreshing the page, visiting your personal /stats page, and every count will be accurate.
When signed-out and make 5 classification, the UI displays the authentication invitation banner (DMP is an easy one to classify not in demo mode).
When signed-in, making 5 classifications triggers a network request for your project_preferences to check for a new assigned workflow id (you can see this in the network tab on any project).

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

Tests are passing locally and on Github
Documentation is up to date and changelog has been updated if appropriate
You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

All pages of a FEM project load: Home Page, Classify Page, and About Pages
Can submit a classification
- When you submit a classification, the user stats increment as expected
Can sign-in and sign-out
The component is accessible
- Can be used with a screen reader BBC guide to VoiceOver
- Can be used from the keyboard WebAIM guide to keyboard testing
- It is passing accessibility checks in the storybook

Bug Fix

The PR creator has listed user actions to use when testing if bug is fixed
The bug is fixed
Unit tests are added or updated

Refactoring

The PR creator has described the reason for refactoring
The refactored component(s) continue to work as expected

Post-merge

This PR adds translations keys to English dictionary(s). See guidance for pulling new keys to Lokalise here.

coveralls · 2024-11-15T23:16:27Z

coverage: 77.635% (-0.2%) from 77.843%
when pulling c59c58b on your-project-stats-refactor
into 74d5ede on master.

goplayoutside3 · 2024-11-20T02:12:10Z

packages/app-project/public/locales/en/screens.json

-    "FinishedForTheDay": {
-      "buttons": {
-        "stats": "See the stats"
-      },
-      "text": "Your answers are saved for the research team while you're working. See the project stats and return to the {{projectName}} home page.",
-      "title": "Finished for the day?",
-      "ProjectImage": {
-        "alt": "Image for {{projectName}}"
-      }
-    },


This component is deleted for good, so its translation keys are deleted throughout.

goplayoutside3 · 2024-11-20T02:12:53Z

packages/app-project/public/locales/en/screens.json

-      "todaysCount": "Classifications today",
-      "totalCount": "Classifications total",
-      "text": "Keep up the great work!",
-      "title": "Your {{projectName}} statistics",
-      "DailyClassificationsChart": {
-        "title": "{{projectName}} daily classification counts"
-      }
+      "allTime": "All Time",
+      "lastSeven": "Last 7 Days",
+      "link": "See more",
+      "title": "Your Stats"


I'm able to reuse "title" in all languages, but the rest of the translation keys will be modified and/or deleted in Lokalise post-merge.

goplayoutside3 · 2024-11-20T02:14:20Z

packages/app-project/src/screens/ClassifyPage/components/ClassifierWrapper/ClassifierWrapper.js

-  /* Only increment stats on the classify page if the subject is not retired or not already seen by current user */
-  const incrementStats = yourStats?.increment
+  /*
+    Increment user stats on every classification submitted.
+    Add the recently classified subject to the user's Recents.
+  */


ERAS counts all classifications as a +1 regardless of subject status as 'already seen' or 'retired'. Therefore, we want to increment user stats after every submitted classification.

goplayoutside3 · 2024-11-20T02:17:55Z

packages/app-project/src/screens/ClassifyPage/components/ClassifierWrapper/ClassifierWrapper.js

+    Add the recently classified subject to the user's Recents.
+  */
+  const projectID = project?.id
+  const { mutate } = useYourProjectStats({ projectID, userID })


The mutate function is referenced in the ClassifierWrapper component because the useYourProjectStats() hook must be called in a functional component. mutate is specific to the SWR key in useYourProjectStats(), so it can be used to update the data returned by the hook on every classification submitted.

goplayoutside3 · 2024-11-20T02:20:16Z

.../app-project/src/screens/ClassifyPage/components/ClassifierWrapper/ClassifierWrapper.spec.js

-      describe('with a retired subject', function () {
+      describe('with user signed in and any subject', function () {


These ClassifierWrapper tests are modified to "with user signed in and any subject" because ERAS counts all classifications as a +1 regardless of subject status as 'already seen' or 'retired'. Therefore, the behavior of user recents and user stats is the same in each scenario.

I also didn't include a test here for "increments stats on classification" because this spec file is written with implementation patterns and enzyme, and "increments stats on classification" requires looking at actual UI.

goplayoutside3 · 2024-11-20T02:21:28Z

...ect/src/screens/ClassifyPage/components/ClassifierWrapper/ClassifierWrapperConnector.spec.js

-      it('should include your personal stats', function () {
-        expect(wrapper.props().yourStats).to.equal(mockStore.user.personalization)
-      })


user.personalization does not always equal just the "your personal stats" feature. Stats used to be a part of the UserPersonalization store, but that's not the case in this PR. Specs that equate personalization to stats have been removed.

packages/app-project/src/screens/ClassifyPage/components/RecentSubjects/RecentSubjects.js

goplayoutside3 · 2024-11-20T02:23:04Z

packages/app-project/src/screens/ClassifyPage/components/YourProjectStats/YourProjectStats.js

+function Stat({ label = '', value = 0 }) {
+  return (
+    <Box>
+      <SpacedText textAlign='center'>{label}</SpacedText>
+      <Text
+        color={{ light: 'neutral-1', dark: 'accent-1' }}
+        size='xxlarge'
+        textAlign='center'
+      >
+        {/* Insert commmas where appropriate */}
+        {value.toLocaleString()}
+      </Text>
+    </Box>
+  )
+}


This Stat presentational component is purposely different than the @shared/components/Stat in app-project. The styling here matches lib-user, and the lack of animation is also intentional.

goplayoutside3 · 2024-11-20T02:25:15Z

...pp-project/src/screens/ClassifyPage/components/YourProjectStats/YourProjectStatsContainer.js

+/**
+ * This is a relatively simple container for ProjectStats, but data fetching
+ * and store observing are purposely separated from the presentational component
+ * styling and logic. Fetching user data requires authorization, making it
+ * complicated to use a mock library like MSW for useYourProjectStats() hook.
+ */


Just noting I considered using MSW in order to see YourProjectStatsContainer in Storybook and unit tests, but mocking authorized requests for user stats was too challenging for the return.

If it's of any use, the SubjectPicker stories have an example of mocking an authenticated request to Panoptes with MSW.

You don't need a signed-in user to make requests in the SubjectPicker do you? 🤔 ERAS user stats requests require authorization from the signed-in user. User authorization is what I had a difficult time mocking for YourProjectStatsContainer.

The subject picker makes an auth'ed request to get the already seen and retired status of each subject for your user account. I don't know if this will help here, but in the subject picker you can ignore the auth headers and mock the endpoint directly. This is the auth'ed API query:

front-end-monorepo/packages/app-project/src/shared/components/SubjectPicker/SubjectPicker.stories.js

Lines 41 to 49 in 1862ade

http.get(`${PANOPTES_HOST}/api/subjects/selection`, ({ request }) => {

return HttpResponse.json({

subjects: [

{ id: 1, already_seen: false, retired: false },

{ id: 2, already_seen: true, retired: false },

{ id: 3, already_seen: true, retired: true }

]

})

}),

Gotcha, that makes much more sense! In YourProjectStatsContainer I started mocking all the requests in useYourProjecStats() hook, got to the authorization part, and said 'uh oh' better just separate the presentational component into its own stories. I'm going to leave this PR as is, but definitely want to use MSW more in the future.

TBH, if your container passes the ERAS response directly into the presentation component, then you're already testing with mock API data anyway. The subject picker transforms the API response, so mocking tests that the transformation is working.

goplayoutside3 · 2024-11-20T02:27:52Z

...app-project/src/screens/ClassifyPage/components/YourProjectStats/helpers/dateRangeHelpers.js

@@ -0,0 +1,21 @@
+/* Today in UTC */
+export function getTodayDateString() {


For the helper functions in this file, I considered importing similar functions from lib-user, but the hooks and helpers in lib-user are specific to components that handle stats counts and hours and multiple date ranges. This app-project YourProjectStats doesn't need all the bells and whistles like lib-user, so I decided to define simplified helper functions here.

goplayoutside3 · 2024-11-20T02:28:57Z

...s/app-project/src/screens/ClassifyPage/components/YourProjectStats/helpers/incrementStats.js

@@ -0,0 +1,19 @@
+export default function incrementStats(mutate, projectID, userID) {


mutate is specific to useYourProjectStats() hook. No API calls are made here. The data returned by the custom hook is just updated per classification submitted.

goplayoutside3 · 2024-11-20T02:30:41Z

...ages/app-project/src/screens/ClassifyPage/components/YourProjectStats/useYourProjectStats.js

+  // only fetch stats when a userID is available. Don't fetch if no user logged in.
+  const key = authorization && userID ? { endpoint, projectID, userID, authorization } : null
+  return useSWR(key, fetchStats, SWROptions)


This useSWR hook will not run with a user is not signed in.

goplayoutside3 · 2024-11-20T02:33:06Z

packages/app-project/stores/Store.spec.js

@@ -56,18 +56,14 @@ describe('Stores > Store', function () {
  })

  describe('when a user signs out', function () {
-    it('should reset the user project preferences and stats', function () {
+    it('should reset the user project preferences and sessionCount', function () {


sessionCount was previously tied up with user stats in app-project's store. I've completely separated the two in this PR, so there are lots of spec file changes to accommodate.

Session count is used to display the AuthenticationInvitation component when a user is not signed in. It's also used to refetch a user's assigned workflow id every 5 classifications they submit. No effect on user classification stats.

goplayoutside3 · 2024-11-20T02:33:56Z

packages/app-project/stores/User/User.spec.js

-        { activity_count: 23 }
+        {
+          links: {
+            user: '1'
+          }


activity_count is not used for anything in FEM anymore, so don't mock it anywhere.

goplayoutside3 · 2024-11-21T19:06:36Z

packages/lib-classifier/src/components/Classifier/ClassifierContainer.js

@@ -250,7 +250,7 @@ export default function ClassifierContainer({
            subjectID={subjectID}
            workflowSnapshot={workflowSnapshot}
          /> :
-          <Paragraph>Loading…</Paragraph>
+          <Paragraph>Loading the Classifier</Paragraph>


This is just to differentiate "loading" messages between app-project loading the Classify page, lib-classifier loading the entire Classifier component, and parts of the Classifier that display messages like "Loading a subject".

Eventually the loading UX of lib-classifier will be enhanced with skeleton components, loading animations, polite live regions, etc. See #6484

goplayoutside3 · 2024-11-21T19:08:04Z

packages/app-project/src/screens/ClassifyPage/components/RecentSubjects/RecentSubjects.js

-    <ContentBox title={t('Classify.RecentSubjects.title', { projectName })}>
-      <Paragraph margin={{ top: 'none' }}>
-        {t('Classify.RecentSubjects.text')}
-      </Paragraph>
+    <ContentBox title={t('Classify.RecentSubjects.title')}>


Removal of the Paragraph is per designer request, and the projectName variable was never actually used anywhere in this component. It's removed from the translation dictionaries too.

goplayoutside3 · 2024-11-21T19:09:22Z

packages/app-project/src/screens/ClassifyPage/ClassifyPage.js

@@ -89,7 +100,7 @@ function ClassifyPage({
          gap='medium'
          pad='medium'
        >
-          <Box as='main' fill='horizontal'>
+          <Box as='main' height={{ min: '400px'}} width='100%'>


Gave the classifier area a min-height so the layout jump is not so jarring once lib-classifier is actually imported and displayed. The Classifier loading UX will eventually be enhanced. See #6484

goplayoutside3 · 2024-11-21T19:30:52Z

@seanmiller26 after our design chat today I applied as many suggestions as possible. It was more complicated to change the number of subject cards at certain breakpoints than I anticipated, so I left the two options as 3 or 1. However, I was able to adjust the overall container layout so the stats text is never squished, and was able to reduce the squishing of recent subject cards, too.

Here's a video of what the changes look like for now, and I definitely think more enhancements can be made once a universal subject card is available for use throughout FEM.

recording.mov

seanmiller26 · 2024-11-21T19:37:12Z

We talked in a Slack huddle and all the styling looks good!

goplayoutside3 · 2024-11-21T19:41:27Z

packages/app-project/src/hooks/usePanoptesAuth.js

+    auth.checkCurrent()
    const token = await auth.checkBearerToken()


This change is inspired by #6345. auth.checkCurrent() is required to resolve before auth.checkBearerToken() can be called. Without this pattern, there was sometimes a race condition where a user's stats displayed as 0 : 0.

You’ll need await auth.checkCurrent() here, otherwise the token can be empty for a signed-in user account.

front-end-monorepo/packages/lib-react-components/src/hooks/useUnreadMessages.js

Lines 20 to 21 in cb221b9

await auth.checkCurrent()

const token = await auth.checkBearerToken()

Now that I look at this hook again, it's broken. It saves the bearer token in component state, so that it gets reused even after it has expired. This hook won't refresh expired auth tokens. Panoptes refresh tokens are valid for something like 2 weeks, but auth tokens only last for two hours. Your code will work for the first two hours of any browser session, then stop fetching after that.

That's my fault. I think I wrote this before I properly understood how OAuth works.

useAuthToken, in the user library, will refresh tokens when they expire, or are about to expire. It's a variation on the usePrevious pattern. It returns the current, valid auth token, while silently refreshing a stale value in the background.

front-end-monorepo/packages/lib-user/src/hooks/usePanoptesAuthToken.js

Lines 1 to 36 in 1d48e37

import auth from "panoptes-client/lib/auth"

import { useState } from "react"

const isBrowser = typeof window !== "undefined"

let defaultToken

/*

See comments in https://github.com/zooniverse/front-end-monorepo/pull/6345

Top-level await in modules has been supported in Node

and in all browsers since 2021. However, ES modules are still

not supported in the monorepo. An immediately-invoked async

function is a workaround when top-level await is not supported.

https://v8.dev/features/top-level-await

*/

(async function getDefaultToken() {

defaultToken = null

if (isBrowser) {

await auth.checkCurrent()

defaultToken = await auth.checkBearerToken()

}

})()

export default function usePanoptesAuthToken() {

const [token, setToken] = useState(defaultToken)

async function fetchPanoptesAuthToken() {

await auth.checkCurrent()

const newToken = await auth.checkBearerToken()

if (newToken !== token) {

setToken(newToken)

}

}

fetchPanoptesAuthToken()

return token

}

goplayoutside3 added 6 commits November 15, 2024 12:19

create new YourProjectStats component with useSWR for data

4e1f716

create a stats container component and a RequireUser component

015eaa1

create ClassificationsChart component with VisX

0a67bde

fix styling of ClassificationChart

d669f06

remove all the legacy YourStats code

abe26aa

Delete FinishedForTheDay component per designer request

9468468

fix consistent returns warning

6db35bd

This was referenced Nov 15, 2024

Use ERAS for all user stats in YourStats component #6401

Closed

test(app-project): run YourStats tests in a non-UTC timezone #6412

Open

lib-user: Use a getDefaultDateRange() utility #6420

Merged

goplayoutside3 added 4 commits November 19, 2024 14:18

Merge branch 'master'

465891f

add a mutate function for incrementing stats on classification

10a9912

match Figma design and edit translation dictionaries

be27129

write unit tests for YourProjectStats

b8720b7

goplayoutside3 commented Nov 20, 2024

View reviewed changes

packages/app-project/src/screens/ClassifyPage/components/RecentSubjects/RecentSubjects.js Outdated Show resolved Hide resolved

goplayoutside3 commented Nov 20, 2024

View reviewed changes

include mutate in the ClassifierWrapper useCallback

7705b3f

be sure to increment session count on each classification

89251aa

goplayoutside3 mentioned this pull request Nov 20, 2024

fix(app-root): wait for login to complete before fetching authenticated user data #6345

Merged

12 tasks

goplayoutside3 added 2 commits November 20, 2024 11:52

add auth.checkCurrent() to usePanoptesAuth() hook

66ff700

adjust layout of ClassifyPage

d7ec5b0

goplayoutside3 commented Nov 21, 2024

View reviewed changes

goplayoutside3 added 2 commits November 21, 2024 13:11

clean up translation keys

5624eb5

Merge branch 'master' into your-project-stats-refactor

c59c58b

goplayoutside3 marked this pull request as ready for review November 21, 2024 19:25

goplayoutside3 requested a review from a team November 21, 2024 19:31

goplayoutside3 commented Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

app-project: Redesign how your stats are displayed on the classify page #6472

app-project: Redesign how your stats are displayed on the classify page #6472

goplayoutside3 commented Nov 15, 2024 •

edited

Loading

coveralls commented Nov 15, 2024 •

edited

Loading

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024 •

edited

Loading

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

eatyourgreens Nov 22, 2024

goplayoutside3 Nov 22, 2024

eatyourgreens Nov 22, 2024 •

edited

Loading

goplayoutside3 Nov 22, 2024

eatyourgreens Nov 22, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 20, 2024 •

edited

Loading

goplayoutside3 Nov 20, 2024

goplayoutside3 Nov 21, 2024 •

edited

Loading

goplayoutside3 Nov 21, 2024 •

edited

Loading

goplayoutside3 Nov 21, 2024

goplayoutside3 commented Nov 21, 2024

seanmiller26 commented Nov 21, 2024

goplayoutside3 Nov 21, 2024 •

edited

Loading

eatyourgreens Nov 21, 2024 •

edited

Loading

eatyourgreens Nov 22, 2024 •

edited

Loading

eatyourgreens Nov 22, 2024 •

edited

Loading

		describe('with a retired subject', function () {
		describe('with user signed in and any subject', function () {

	http.get(`${PANOPTES_HOST}/api/subjects/selection`, ({ request }) => {
	return HttpResponse.json({
	subjects: [
	{ id: 1, already_seen: false, retired: false },
	{ id: 2, already_seen: true, retired: false },
	{ id: 3, already_seen: true, retired: true }
	]
	})
	}),

		@@ -0,0 +1,21 @@
		/* Today in UTC */
		export function getTodayDateString() {

		@@ -0,0 +1,19 @@
		export default function incrementStats(mutate, projectID, userID) {

		auth.checkCurrent()
		const token = await auth.checkBearerToken()

	await auth.checkCurrent()
	const token = await auth.checkBearerToken()

	import auth from "panoptes-client/lib/auth"
	import { useState } from "react"

	const isBrowser = typeof window !== "undefined"

	let defaultToken
	/*
	See comments in https://github.com/zooniverse/front-end-monorepo/pull/6345
	Top-level await in modules has been supported in Node
	and in all browsers since 2021. However, ES modules are still
	not supported in the monorepo. An immediately-invoked async
	function is a workaround when top-level await is not supported.
	https://v8.dev/features/top-level-await
	*/
	(async function getDefaultToken() {
	defaultToken = null
	if (isBrowser) {
	await auth.checkCurrent()
	defaultToken = await auth.checkBearerToken()
	}
	})()

	export default function usePanoptesAuthToken() {
	const [token, setToken] = useState(defaultToken)

	async function fetchPanoptesAuthToken() {
	await auth.checkCurrent()
	const newToken = await auth.checkBearerToken()
	if (newToken !== token) {
	setToken(newToken)
	}
	}

	fetchPanoptesAuthToken()
	return token
	}

app-project: Redesign how your stats are displayed on the classify page #6472

Are you sure you want to change the base?

app-project: Redesign how your stats are displayed on the classify page #6472

Conversation

goplayoutside3 commented Nov 15, 2024 • edited Loading

Package

Linked Issue and/or Talk Post

Describe your changes

How to Review

Checklist

General

General UX

Bug Fix

Refactoring

Post-merge

coveralls commented Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goplayoutside3 Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eatyourgreens Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goplayoutside3 Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goplayoutside3 Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

goplayoutside3 Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goplayoutside3 commented Nov 21, 2024

seanmiller26 commented Nov 21, 2024

goplayoutside3 Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

eatyourgreens Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

eatyourgreens Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

eatyourgreens Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

goplayoutside3 commented Nov 15, 2024 •

edited

Loading

coveralls commented Nov 15, 2024 •

edited

Loading

goplayoutside3 Nov 20, 2024 •

edited

Loading

eatyourgreens Nov 22, 2024 •

edited

Loading

goplayoutside3 Nov 20, 2024 •

edited

Loading

goplayoutside3 Nov 21, 2024 •

edited

Loading

goplayoutside3 Nov 21, 2024 •

edited

Loading

goplayoutside3 Nov 21, 2024 •

edited

Loading

eatyourgreens Nov 21, 2024 •

edited

Loading

eatyourgreens Nov 22, 2024 •

edited

Loading

eatyourgreens Nov 22, 2024 •

edited

Loading