Skip to content

Conversation

@KATIETOLER
Copy link
Contributor

@KATIETOLER KATIETOLER commented Jun 26, 2025

This is the first step in adding highlighting related activities in the event timeline/summary row.

when hovering on an activity event row, a user can see a visual indicator on the related event rows
Screenshot 2025-06-27 at 12 27 46 PM
Screenshot 2025-06-27 at 12 27 59 PM


Next steps:

  • do the same thing for workflows
  • slight dim on the other event content

@KATIETOLER KATIETOLER requested a review from rossedfort as a code owner June 26, 2025 17:56
@vercel
Copy link

vercel bot commented Jun 26, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
holocene ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2025 7:47pm

@KATIETOLER KATIETOLER requested review from Alex-Tideman and andrewzamojc and removed request for rossedfort June 26, 2025 17:56
Copy link
Contributor

@andrewzamojc andrewzamojc left a comment

Choose a reason for hiding this comment

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

Code looks pretty good to me! I'll try it shortly.

tr[data-testid='event-summary-row'].active:hover {
@apply border-l-4 border-slate-600 bg-slate-300;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Any reason not to do this with tailwind? Instead of conditionally applying the .active class and styling it here, you could probably modify you're first class entry

class={[
  // existing
  'hover:cursor-pointer',
  // new
  hasRelatedActivities(group, hoveredEventId) && 'bg-slate-800 hover:bg-slate-300 etc'
]}

or something like that.

/>
{:else}
<EventSummaryRow
bind:hoveredEventId
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment: Pretty slick solution.

@Alex-Tideman
Copy link
Collaborator

This is looking great!

One suggestion, I would try to flip the hover backgrounds so the background of the row you are currently hovering is the brighter background, and the related backgrounds are the more subtle ones. See how it looks.

Screenshot 2025-06-26 at 2 43 10 PM

@Alex-Tideman
Copy link
Collaborator

We'll want a different color too for light mode

Screenshot 2025-06-26 at 2 49 10 PM

Copy link
Contributor

@andrewzamojc andrewzamojc left a comment

Choose a reason for hiding this comment

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

Works great! LGTM pending the light mode comment from Alex.

@KATIETOLER KATIETOLER self-assigned this Jun 26, 2025
@Alex-Tideman
Copy link
Collaborator

Looking awesome! Couple of tiny little things:

I would have all rows be the interactive bg on hover, it's a little confusing when they are different colors depending on what the row is (workflow task doesn't have the interactive bg for example)

On light mode, let's make the event id white text on row hover. all the other text is white on hover except that.

RowHighlighting.mov

@Alex-Tideman Alex-Tideman merged commit 9da1bd8 into main Jun 30, 2025
16 checks passed
@Alex-Tideman Alex-Tideman deleted the esr-highlight-related/kt branch June 30, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants