Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

DEVPROD-2637 Add Guide Cue for Parsley viewable log files #2193

Merged
merged 8 commits into from
Dec 19, 2023

Conversation

khelif96
Copy link
Contributor

DEVPROD-2637

Description

This PR adds a guide cue to point users to the Open in Parsley button for task-uploaded files. The guide cue is only shown on the first visible Parsley button.
There is a scroll effect that accomplishes 2 things.

  1. It surfaces the button if the button is not immediately visible.
  2. It avoids a weird bug that causes the guide cue to draw a white background when rendering over a non-visible element.

Screenshots

image

@khelif96 khelif96 requested a review from a team December 13, 2023 20:49
Copy link

cypress bot commented Dec 13, 2023

Passing run #14723 ↗︎

0 541 10 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

CR
Project: Spruce Commit: 72af0b8519
Status: Passed Duration: 24:33 💡
Started: Dec 19, 2023 9:39 PM Ended: Dec 19, 2023 10:04 PM

Review all test suite changes for PR #2193 ↗︎

Copy link
Contributor

@SupaJoon SupaJoon left a comment

Choose a reason for hiding this comment

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

Can you comment on the guide cue background fix or open an LG ticket if it's something they can fix?

Comment on lines 22 to 23
triggerRef: React.RefObject<HTMLAnchorElement>;
index: number;
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
triggerRef: React.RefObject<HTMLAnchorElement>;
index: number;
guideCueTriggerRef: React.RefObject<HTMLAnchorElement>;
firstParsleyFileIndex: number;

Comment on lines +97 to +100
const firstParsleyFileIndex = useMemo(
() => files.findIndex((file) => file.urlParsley !== null),
[files]
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the useMemo is overkill here. The computation result is a number as opposed to a reference. This component only has two props and one of which is cached in Apollo. And there is only 1 truly stateful variable (useState)

@khelif96 khelif96 merged commit cc5b551 into evergreen-ci:main Dec 19, 2023
6 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants