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

DEVPROD-10205: Create onboarding tutorial #590

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

minnakt
Copy link
Contributor

@minnakt minnakt commented Jan 27, 2025

DEVPROD-10205

Description

This PR adds the onboarding tutorial. Heavily referenced from preexisting Cloud work 🙏

Backdrop was my idea though... let me know if you don't like it

Assumptions

The user views a project that has activated tasks in the first build variant. It's hard to place a guide cue on the task box if that's not the case.

Note that older projects literally have no tasks on their waterfall due to the task collection TTL. 😅

Screenshots

I wasn't sure how the restart walkthrough functionality should look on the UI, I just put a button there for now. I can talk to Will about it.

Screen.Recording.2025-01-26.at.8.17.21.PM.mov

Testing

  • Cypress tests
  • Jest tests

@minnakt minnakt added the spruce label Jan 27, 2025
@minnakt minnakt marked this pull request as ready for review January 27, 2025 14:48
@minnakt minnakt requested a review from a team as a code owner January 27, 2025 14:48
Copy link
Contributor

@sophstad sophstad left a comment

Choose a reason for hiding this comment

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

Really nice approach! Thank u mms 🫶

const currentStepRef = useRef<HTMLElement | null>(null);

// Exposes a function via the ref to restart the walkthrough.
useImperativeHandle(ref, () => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

{
title: "New Layout",
description:
"We've changed the Project Health page to a new layout for increased information density. Columns are commits, rows are build variants, and tasks are box icons.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Switch order for consistency. Also maybe "squares" instead of "box icons"?

Suggested change
"We've changed the Project Health page to a new layout for increased information density. Columns are commits, rows are build variants, and tasks are box icons.",
"We've changed the Project Health page to a new layout for increased information density. Columns are commits, rows are build variants, and squares are tasks.",

@@ -91,7 +94,7 @@ export const TaskStatusIconLegend: React.FC = () => {
useOnClickOutside([buttonRef, popoverRef], () => setOpen(false));

return (
<div>
<div {...legendProps}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to open the legend when the guide cue is here? May crowd the page too much but just an idea. It feels awk that the button is not clickable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok turns out it wasn't that hard to do this i just had to strategically put some walkthrough steps in a different order

but the task legend one looked awkward (because the legend just gets eclipsed by the guide cue) so i left it out 👀 i added it to other elements!

{
title: "Pin Build Variants",
description:
"Pin your favorite build variants at the top to help with debugging and monitoring workflows.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"Favorite" feels like a strong word... I ❤️ Shared Library Enterprise RHEL 8

Suggested change
"Pin your favorite build variants at the top to help with debugging and monitoring workflows.",
"Pin variants to the top of the page to help with debugging and monitoring common workflows.",

currentStep={currentStepIdx + 1}
data-cy="walkthrough-guide-cue"
numberOfSteps={walkthroughSteps.length}
onDismiss={() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dismissed the walkthrough without finishing it, but when I reloaded the page it started again which feels aggressive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i wasn't able to replicate,,, let me know if you're still seeing 🥺

{
title: "Summary View",
description:
"Introducing an alternative to the charts — view a summary of the task statuses for any given run.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Introducing an alternative to the charts — view a summary of the task statuses for any given run.",
"An alternative to project health charts — view a summary of task statuses for any given run.",

description:
"Introducing an alternative to the charts — view a summary of the task statuses for any given run.",
targetId: "summary-view",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Might we add a cue for the "Search by git hash" feature? I think users called it out as harder to find in UAT, and ... menus can get buried easily.

Comment on lines 125 to 133
let firstActiveTaskId = "";
for (let i = 0; i < buildVariants[0]?.builds?.length; i++) {
const { builds } = buildVariants[0];
if (builds[i].tasks.length > 0) {
firstActiveTaskId = builds[i].tasks[0].id;
break;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it perhaps make more sense to calculate this in the BuildRow component if isFirstBuild is true?

{
title: "New Layout",
description:
"We've changed the Project Health page to a new layout for increased information density. Columns are commits, rows are build variants, and tasks are box icons.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think it makes sense to not imply we're changing the project health page. This is a new page, no need to mention it.

Suggested change
"We've changed the Project Health page to a new layout for increased information density. Columns are commits, rows are build variants, and tasks are box icons.",
"We've introduced a new layout for increased information density. Columns are commits, rows are build variants, and tasks are box icons.",

@minnakt minnakt requested a review from sophstad January 27, 2025 22:41
{
title: "Search by Git Hash",
description:
"Explore other filtering options in the menu, such as searching by git hash.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Kinda a hacky change, but I think worth it: if we shorten the guide text here, then the cue changes orientation and the menu is visible!

Suggested change
"Explore other filtering options in the menu, such as searching by git hash.",
"Explore other filtering options in the menu.",
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops idk how i missed that 🫣

Comment on lines +73 to +75
if (nextStep.shouldClick) {
nextTargetElement.click();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This works so well 👏

Comment on lines +53 to +59
<Button
data-cy="restart-walkthrough-button"
onClick={() => guideCueRef.current?.restart()}
size={ButtonSize.XSmall}
>
Restart walkthrough
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hrmmmm I would love to show this button and launch the walkthrough only once the beta period starts. Is that doable now or is it dependent on some of the other tix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think it might be simpler if i merge it on friday or monday...? 🤔 im on deploy so i can wait for whatever dependencies and deploy next week if that makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense to me! We can throw a do-not-merge label on here

@minnakt minnakt requested a review from sophstad January 28, 2025 18:40
Copy link
Contributor

@sophstad sophstad left a comment

Choose a reason for hiding this comment

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

😍

Comment on lines +53 to +59
<Button
data-cy="restart-walkthrough-button"
onClick={() => guideCueRef.current?.restart()}
size={ButtonSize.XSmall}
>
Restart walkthrough
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense to me! We can throw a do-not-merge label on here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants