Adds invitation for participants to view the new progress view #58372

kobryan0619 · 2024-05-02T03:11:08Z

This PR creates a modal inviting users to use the new section progress view. This PR:

Creates the pop-up with the right elements.
Allows the user to dismiss the pop-up or accept the invitation
Logs the events to amplitude.
Save data that the invitation has been seen or delayed.
Accurately process a "delayed" invitation

(NEW): It DOES

"Remember" the status of the invitation if a user switches between the tabs in the dashboard.

Links

Ticket

Testing story

Tests coming in separate PR.

Deployment strategy

Follow-up work

Fix issue with tabs not recognizing the data.
fix issue with image being slow to load

MollyPeredo · 2024-05-02T13:53:29Z

Looks great! Just one tweak - can you remove the teal border-radius on the "close" button?

kobryan0619 · 2024-05-02T14:30:22Z

Looks great! Just one tweak - can you remove the teal border-radius on the "close" button?

Screen.Recording.2024-05-02.at.9.29.28.AM.mov

That's actually a focus indicator that moves through out the component as a user tab navigates. This outline is also what shows up in other modals - is there a different focus indicator you'd prefer for this?

dju90 · 2024-05-02T14:36:55Z

It does not: "Remember" the status of the invitation if a user switches between the tabs in the dashboard. It DOES however remember the status of the invitation if a teacher reloads or logs out and back in.

Can you clarify what this means? Does it mean that if I click "close" on the invitation, then switch to a different dashboard tab and then switch back, the invitation will re-appear?

lfryemason

Looks great, a couple nit-picks and that's it. Code structure and logic are all sound!

lfryemason · 2024-05-02T14:33:21Z

apps/src/templates/currentUserRedux.js

@@ -213,6 +215,9 @@ export default function currentUser(state = initialState, action) {
      progressTableV2ClosedBeta: progress_table_v2_closed_beta,
      isLti: is_lti,
      isTeacher: user_type === UserTypes.TEACHER,
+      dateProgressTableInvtationDelayed:


nit: Invtation -> Invitation

Suggested change

dateProgressTableInvtationDelayed:

dateProgressTableInvitationDelayed:

lfryemason · 2024-05-02T14:35:01Z

apps/src/templates/sectionProgress/SectionProgress.jsx

@@ -174,6 +179,12 @@ class SectionProgress extends Component {
        className={dashboardStyles.dashboardPage}
        data-testid="section-progress-v1"
      >
+        {allowUserToSelectV2View && (


Nit: could we move this logic to the SectionProgressSelector? That class contains all the logic for the banners and determines whether to show v1 or v2. I feel like this popup fits better there.

lfryemason · 2024-05-02T14:46:33Z

apps/src/templates/sectionProgressV2/InviteToV2ProgressModal.jsx

+    });
+  };
+
+  if (invitationOpen) {


Optional nit: I find if (!invitationOpen) { return null} ... to be slightly clearer because it shows the early return before the big blob of content to show. Completely optional nit pick

MollyPeredo · 2024-05-02T16:35:34Z

Looks great! Just one tweak - can you remove the teal border-radius on the "close" button?

Screen.Recording.2024-05-02.at.9.29.28.AM.mov
That's actually a focus indicator that moves through out the component as a user tab navigates. This outline is also what shows up in other modals - is there a different focus indicator you'd prefer for this?

Ahhhhh, got it. This is perfect then, thanks!

lfryemason

Fantastic! A few optional nits, but I have no problem merging as is!

lfryemason · 2024-05-07T21:40:00Z

apps/src/templates/sectionProgressV2/InviteToV2ProgressModal.jsx

+
+function InviteToV2ProgressModal({
+  sectionId,
+


Suggested change

nit

lfryemason · 2024-05-07T21:42:58Z

apps/src/templates/sectionProgressV2/InviteToV2ProgressModal.jsx

+function InviteToV2ProgressModal({
+  sectionId,
+
+  // from redux


Hmm, I have complicated thoughts about this kind of comment. On one hand I can see how it would be useful to the reader, on the other, these kinds of comments tend to get stale and unreliable very quickly.

Do you see value in keeping this in the codebase long term or was this better for your initial iteration?

I think my personal preference is to remove it, especially since there is already a list of props that are from redux in the connect function at the bottom of the file. But, I'll leave it up to you as an optional nit.

Yeah, this is probably another place where it would be nice, as an org/team to get on the same page. I see this from redux comment kinda a lot in our codebase. I don't know if it is just because it is the "codebase I grew up with" or if there is a bigger meaning here, but I actually like the separation between "here's the props passsed in from another component" distinction.

lfryemason · 2024-05-07T21:44:08Z

apps/src/templates/sectionProgressV2/InviteToV2ProgressModal.jsx

+      const startingDate = new Date(dateProgressTableInvitationDelayed);
+      const today = new Date();
+      const differenceInMilliseconds = today.getTime() - startingDate.getTime();
+      const differenceInDays = differenceInMilliseconds / (1000 * 3600 * 24);


Suggested change

const differenceInDays = differenceInMilliseconds / (1000 * 3600 * 24);

const differenceInDays = differenceInMilliseconds / MILLISECONDS_IN_ONE_DAY;

Optional nit: move this to a constant with a meaningful name.

lfryemason · 2024-05-07T21:44:47Z

apps/src/templates/sectionProgressV2/InviteToV2ProgressModal.jsx

+  const [invitationOpen, setInvitationOpen] = React.useState(false);
+
+  React.useEffect(() => {
+    const timeSinceInvitationLastDelayed = () => {


Suggested change

const timeSinceInvitationLastDelayed = () => {

const numDaysSinceInvitationLastDelayed = () => {

Optional nit.

added all changes

d31c3c6

kobryan0619 marked this pull request as ready for review May 2, 2024 03:11

kobryan0619 requested a review from a team as a code owner May 2, 2024 03:11

kobryan0619 requested review from lfryemason, dju90 and MollyPeredo May 2, 2024 11:35

modified tests

38d515a

lfryemason reviewed May 2, 2024

View reviewed changes

kobryan0619 added 2 commits May 2, 2024 10:40

maybe fixed tab switching issue

a39e7f1

added in logic for delayed invite

a0bf63d

kobryan0619 added 4 commits May 2, 2024 11:59

incorporated liams feedback

e8607d9

Merge branch 'staging' into invitation-for-V2-progress-view

955b40c

updated

7a8d19b

fixed delayed invite

5e0d62a

kobryan0619 requested a review from lfryemason May 3, 2024 15:18

kobryan0619 added 5 commits May 6, 2024 12:10

polish pieces

ec7400a

Merge branch 'staging' into invitation-for-V2-progress-view

53eebaf

ready for review

6587acc

added tracking

c5ea6cf

added message

df5888f

lfryemason approved these changes May 7, 2024

View reviewed changes

kobryan0619 merged commit fb27c28 into staging May 8, 2024
2 checks passed

kobryan0619 deleted the invitation-for-V2-progress-view branch May 8, 2024 17:20

kobryan0619 mentioned this pull request May 9, 2024

Added tests for Progress View V2 Invitation #58471

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds invitation for participants to view the new progress view #58372

Adds invitation for participants to view the new progress view #58372

kobryan0619 commented May 2, 2024 •

edited

Loading

MollyPeredo commented May 2, 2024

kobryan0619 commented May 2, 2024

dju90 commented May 2, 2024 •

edited

Loading

lfryemason left a comment

lfryemason May 2, 2024

lfryemason May 2, 2024

lfryemason May 2, 2024

MollyPeredo commented May 2, 2024

lfryemason left a comment

lfryemason May 7, 2024

lfryemason May 7, 2024

kobryan0619 May 8, 2024

lfryemason May 7, 2024

lfryemason May 7, 2024

	dateProgressTableInvtationDelayed:
	dateProgressTableInvitationDelayed:

	const differenceInDays = differenceInMilliseconds / (1000 * 3600 * 24);
	const differenceInDays = differenceInMilliseconds / MILLISECONDS_IN_ONE_DAY;

	const timeSinceInvitationLastDelayed = () => {
	const numDaysSinceInvitationLastDelayed = () => {

Adds invitation for participants to view the new progress view #58372

Adds invitation for participants to view the new progress view #58372

Conversation

kobryan0619 commented May 2, 2024 • edited Loading

Links

Testing story

Deployment strategy

Follow-up work

MollyPeredo commented May 2, 2024

kobryan0619 commented May 2, 2024

dju90 commented May 2, 2024 • edited Loading

lfryemason left a 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

MollyPeredo commented May 2, 2024

lfryemason left a 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

Choose a reason for hiding this comment

kobryan0619 commented May 2, 2024 •

edited

Loading

dju90 commented May 2, 2024 •

edited

Loading