-
Notifications
You must be signed in to change notification settings - Fork 1
Improve Span Details Panel UI and Functionality per design #76
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
Changes from 7 commits
b69eaab
e359cd0
e8175e8
09735e5
83309b7
0dc7574
7dbd140
5509c9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import React, { useState } from 'react'; | ||
import clsx from 'clsx'; | ||
import { IconButton } from '@grafana/ui'; | ||
import { IconButton, Input } from '@grafana/ui'; | ||
|
||
import type { SpanInfo } from '../../types'; | ||
import { mkUnixEpochFromNanoSeconds, formatUnixNanoToDateTime, formatDuration } from 'utils/utils.timeline'; | ||
|
@@ -113,7 +113,7 @@ function ValueWrapper({ | |
}) { | ||
const [tooltip, setTooltip] = useState('Copy value'); | ||
return ( | ||
<span className={`p-2 flex gap-1 items-center justify-between ${italic ? 'italic' : ''}`}> | ||
<span className={`flex gap-1 items-center justify-between ${italic ? 'italic' : ''}`}> | ||
<span className={`px-2 py-2 ${color}`}>{displayValue || value}</span> | ||
<IconButton | ||
name="copy" | ||
|
@@ -234,7 +234,10 @@ export const SpanDetailPanel = ({ | |
); | ||
|
||
const rowClassName = (index: number) => { | ||
return clsx('leading-7', index % 2 === 0 ? 'bg-gray-100 dark:bg-gray-800' : 'bg-gray-50 dark:bg-gray-700'); | ||
return clsx( | ||
'text-[0.9rem] hover:bg-neutral-200 dark:hover:bg-neutral-800', | ||
index % 2 === 0 ? 'bg-neutral-100 dark:bg-neutral-900' : 'bg-white dark:bg-black' | ||
); | ||
}; | ||
|
||
const { spanAttributes, events, resourceAttributes } = React.useMemo(() => { | ||
|
@@ -258,94 +261,126 @@ export const SpanDetailPanel = ({ | |
}, [result, debouncedSearch]); | ||
|
||
return ( | ||
<div className="z-10"> | ||
<div className="overflow-hidden text-sm"> | ||
<h2 className="uppercase">Span Details</h2> | ||
<input type="text" placeholder="Search" value={search} onChange={(e) => setSearch(e.target.value)} /> | ||
<table className="w-full"> | ||
<div className="z-10 overflow-hidden text-sm"> | ||
<div className="px-2 py-2"> | ||
<div className="flex flex-col gap-4 items-start justify-between py-4 px-2 mx-4"> | ||
<div className="flex items-center justify-between gap-2 w-full"> | ||
<span className="uppercase text-lg font-light">Span Details</span> | ||
<IconButton | ||
size="xxl" | ||
name="times" | ||
variant="secondary" | ||
aria-label="Close" | ||
onClick={onClose} | ||
className="w-4 h-4" | ||
/> | ||
</div> | ||
<Input | ||
className="w-full" | ||
type="text" | ||
placeholder="Search details" | ||
value={search} | ||
onChange={(e) => setSearch(e.currentTarget.value)} | ||
suffix={ | ||
search ? ( | ||
<IconButton name="times" variant="secondary" aria-label="Clear" onClick={() => setSearch('')} /> | ||
) : ( | ||
<IconButton name="search" variant="secondary" aria-label="Search" /> | ||
) | ||
} | ||
/> | ||
</div> | ||
</div> | ||
<div className="px-4"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add an extra div here? If it only has a single child, why not add the padding there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is so we can extend the line from end to end of a panel. With padding, that line is broken by the padding width There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
<table className="w-full text-[0.8rem]"> | ||
<tbody> | ||
{basicSpanData.map((item, index) => ( | ||
<tr key={item.key} className={rowClassName(index)}> | ||
<td className="font-semibold text-gray-700 dark:text-gray-300 border-r border-gray-300 dark:border-gray-600 w-1/3 mx-4"> | ||
<span className="px-2 py-2">{item.key}</span>{' '} | ||
{/* TODO: padding & margins are overriden to 0 by the global CSS and it is not possible to set it on the td tag */} | ||
<td className="font-regular text-gray-700 dark:text-gray-300 w-1/3 mx-4"> | ||
<span className="px-2 whitespace-nowrap">{item.key}</span>{' '} | ||
</td> | ||
<td>{item.value && <Value value={item.value} />}</td> | ||
<td className="font-light">{item.value && <Value value={item.value} />}</td> | ||
</tr> | ||
))} | ||
</tbody> | ||
</table> | ||
{spanAttributes.length > 0 && ( | ||
<Accordion | ||
title="Additional Span Data" | ||
showToggle={debouncedSearch === ''} | ||
isExpanded={debouncedSearch !== '' || expandedSections.additionalData} | ||
onToggle={() => toggleSection('additionalData')} | ||
> | ||
</div> | ||
{spanAttributes.length > 0 && ( | ||
<Accordion | ||
dcoric marked this conversation as resolved.
Show resolved
Hide resolved
|
||
title="Additional Span Data" | ||
showToggle={debouncedSearch === ''} | ||
isExpanded={debouncedSearch !== '' || expandedSections.additionalData} | ||
onToggle={() => toggleSection('additionalData')} | ||
> | ||
<div className="px-2"> | ||
<table className="w-full"> | ||
<tbody> | ||
{spanAttributes.map(({ key, value }, index) => ( | ||
<tr key={key} className={rowClassName(basicSpanData.length + spanAttributes.length + index)}> | ||
<td className="font-semibold text-gray-700 dark:text-gray-300 border-r border-gray-300 dark:border-gray-600 w-1/3"> | ||
<span className="px-2 py-2">{key}</span> | ||
<tr key={key} className={rowClassName(index)}> | ||
<td className="font-regular text-gray-700 dark:text-gray-300 w-1/3"> | ||
<span className="px-2 whitespace-nowrap">{key}</span> | ||
</td> | ||
<td>{value && <Value value={value} />}</td> | ||
<td className="font-light">{value && <Value value={value} />}</td> | ||
</tr> | ||
))} | ||
</tbody> | ||
</table> | ||
</Accordion> | ||
)} | ||
|
||
{/* Resource Section */} | ||
{resourceAttributes.length > 0 && ( | ||
<Accordion | ||
title="Resource" | ||
showToggle={debouncedSearch === ''} | ||
isExpanded={debouncedSearch !== '' || expandedSections.process} | ||
onToggle={() => toggleSection('process')} | ||
> | ||
</div> | ||
</Accordion> | ||
)} | ||
|
||
{/* Resource Section */} | ||
{resourceAttributes.length > 0 && ( | ||
<Accordion | ||
title="Resource" | ||
showToggle={debouncedSearch === ''} | ||
isExpanded={debouncedSearch !== '' || expandedSections.process} | ||
onToggle={() => toggleSection('process')} | ||
> | ||
<div className="px-2"> | ||
<table className="w-full"> | ||
<tbody> | ||
{resourceAttributes.map(({ key, value }, index) => ( | ||
<tr key={key} className={rowClassName(index)}> | ||
<td className="font-semibold text-gray-700 dark:text-gray-300 border-r border-gray-300 dark:border-gray-600 w-1/3"> | ||
<span className="px-2 py-2">{key}</span> | ||
<td className="font-regular text-gray-700 dark:text-gray-300 w-1/3"> | ||
<span className="px-2 whitespace-nowrap">{key}</span> | ||
</td> | ||
<td>{value && <Value value={value} />}</td> | ||
<td className="font-light">{value && <Value value={value} />}</td> | ||
</tr> | ||
))} | ||
</tbody> | ||
</table> | ||
</Accordion> | ||
)} | ||
|
||
{/* Events Section */} | ||
{events.length > 0 && ( | ||
<Accordion | ||
title="Events" | ||
showToggle={debouncedSearch === ''} | ||
isExpanded={debouncedSearch !== '' || expandedSections.events} | ||
onToggle={() => toggleSection('events')} | ||
> | ||
</div> | ||
</Accordion> | ||
)} | ||
|
||
{/* Events Section */} | ||
{events.length > 0 && ( | ||
<Accordion | ||
title="Events" | ||
showToggle={debouncedSearch === ''} | ||
isExpanded={debouncedSearch !== '' || expandedSections.events} | ||
onToggle={() => toggleSection('events')} | ||
> | ||
<div className="px-2"> | ||
<table className="w-full"> | ||
<tbody> | ||
{events.map((item, index) => ( | ||
<tr key={item.time} className={rowClassName(index)}> | ||
<td className="font-semibold text-gray-700 dark:text-gray-300 border-r border-gray-300 dark:border-gray-600 w-1/3"> | ||
<span className="px-2 py-2"> | ||
<td className="font-regular text-gray-700 dark:text-gray-300 w-1/3"> | ||
<span className="px-2 whitespace-nowrap"> | ||
{/* print the time in seconds since the start of the span with 3 decimal places */} | ||
{((item.time - span.startTimeUnixNano / 1000000) / 1000).toFixed(3)}s | ||
</span> | ||
</td> | ||
<td>{item.value && <Value value={{ stringValue: item.value }} />}</td> | ||
<td className="font-light">{item.value && <Value value={{ stringValue: item.value }} />}</td> | ||
</tr> | ||
))} | ||
</tbody> | ||
</table> | ||
</Accordion> | ||
)} | ||
</div> | ||
</div> | ||
</Accordion> | ||
)} | ||
</div> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't do px-2 py-2, do p-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do a search and replace on this, we have it elsewhere as well.