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

Coaching Page - Add person's name #1116

Merged
merged 4 commits into from
Oct 4, 2024
Merged

Conversation

caleballdrin
Copy link
Contributor

Description

https://secure.helpscout.net/conversation/2724265010/1237546?folderId=7669074

  • Adding the name of the person on the coaching list and detail page.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@caleballdrin caleballdrin requested a review from canac October 3, 2024 22:22
@caleballdrin caleballdrin self-assigned this Oct 3, 2024
@caleballdrin caleballdrin added the Preview Environment Add this label to create an Amplify Preview label Oct 3, 2024
Copy link
Contributor

github-actions bot commented Oct 3, 2024

Preview branch generated at https://helpscout-coaching-name.d3dytjb8adxkk5.amplifyapp.com

Copy link
Contributor

github-actions bot commented Oct 3, 2024

Bundle sizes [mpdx-react]

Compared against e9331a0

Route Size (gzipped) Diff
/accountLists/[accountListId]/coaching 88.65 KB +3.9 KB

Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

Great work! A couple small fixes and this will be ready to go. Can you add a test too that the users are shown?

You can check off the "Missing second name for coached user" line from our functionality review sheet when this merges, because I'm pretty sure it's a duplicate.

src/components/Coaching/LoadCoachingList.test.tsx Outdated Show resolved Hide resolved
src/components/Coaching/CoachingRow/CoachingRow.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

I wasn't thinking the display="inline" was necessary, but I trust you. Thanks for fixing these!

@caleballdrin caleballdrin merged commit 94a3ffc into main Oct 4, 2024
18 checks passed
@caleballdrin caleballdrin deleted the helpscout-coaching-name branch October 4, 2024 19:32
});
});

expect(await findByText('Initiate for Appointment')).toBeInTheDocument();
await waitFor(
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also customize findByText's timeout if you need to: findByText('...', {}, { timeout: 3000 }).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok good to know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants