Skip to content

🎨 Palette: Enhanced Telemetry Timeline UX #21

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.

Sign up for GitHub

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

Draft
igor-holt wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

🎨 Palette: Enhanced Telemetry Timeline UX #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions components/TelemetryTimeline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,35 @@ import {
ResponsiveContainer,
} from 'recharts';

const CustomTooltip = ({ active, payload, label }: any) => {
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

It's good practice to define a type/interface for the CustomTooltip props instead of using any for better type safety and readability. This helps in understanding what properties active, payload, and label are expected to have.

Suggested change
const CustomTooltip = ({ active, payload, label }: any) => {
const CustomTooltip = ({ active, payload, label }: { active?: boolean; payload?: any[]; label?: string | number }) => {

Comment on lines 16 to +17
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The CustomTooltip component uses the any type for its props, which bypasses TypeScript's type safety. Consider defining a proper interface for the tooltip props. Recharts provides TooltipProps type that can be used, or you can define a custom interface specifying the shape of active, payload, and label parameters to improve type safety and code maintainability.

Suggested change
const CustomTooltip = ({ active, payload, label }: any) => {
import type { TooltipProps } from 'recharts';
const CustomTooltip: React.FC<TooltipProps<number, string>> = ({ active, payload, label }) => {

Copilot uses AI. Check for mistakes.
if (active && payload && payload.length) {
const data = payload[0].payload;
return (
<div className="custom-tooltip" style={{ backgroundColor: '#fff', border: '1px solid #ccc', padding: '10px' }}>
<p className="label" style={{ fontWeight: 'bold', marginBottom: '5px' }}>{new Date(label).toUTCString()}</p>
<p className="intro" style={{ margin: 0 }}>{`Event: ${data.event}`}</p>
<p className="intro" style={{ margin: 0 }}>{`Mode: ${data.mode}`}</p>
<p className="desc" style={{ margin: 0, color: '#8884d8' }}>{`Probability: ${data.probability}`}</p>
</div>
);
}
return null;
};

const TelemetryTimeline: React.FC = () => {
const chartData = getProbabilityData();

if (telemetryData.length === 0) {
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

The telemetryData variable is directly accessed here. While it works, it might be more consistent and maintainable to pass telemetryData as a prop to the TelemetryTimeline component, or fetch it within the component if it's dynamic. This makes the component more reusable and testable.

Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The empty state check uses telemetryData.length === 0 but the chart is rendered using chartData from getProbabilityData(). While these are currently equivalent, there's a logical inconsistency. The component should check the same data source that it displays. Consider using chartData.length === 0 for consistency, or checking both if there's a specific reason to check the raw data.

Suggested change
if (telemetryData.length === 0) {
if (chartData.length === 0) {

Copilot uses AI. Check for mistakes.
return (
<div style={{ padding: '20px' }}>
<Card>
<h2>Telemetry Timeline</h2>
<div style={{ textAlign: 'center', padding: '40px', color: '#666' }}>No telemetry data available</div>
</Card>
</div>
);
}

return (
<div style={{ padding: '20px' }}>
<Card>
Expand All @@ -33,9 +59,7 @@ const TelemetryTimeline: React.FC = () => {
stroke="#888"
/>
<YAxis domain={[0, 1]} />
<Tooltip
labelFormatter={(label) => new Date(label).toUTCString()}
/>
<Tooltip content={<CustomTooltip />} />
<Legend />
<Line type="monotone" dataKey="probability" stroke="#8884d8" strokeWidth={2} />
</LineChart>
Expand All @@ -47,16 +71,16 @@ const TelemetryTimeline: React.FC = () => {
<table style={{ width: '100%', borderCollapse: 'collapse' }}>
<thead>
<tr style={{ borderBottom: '2px solid #ddd' }}>
<th style={{ padding: '10px', textAlign: 'left' }}>Log ID</th>
<th style={{ padding: '10px', textAlign: 'left' }}>UTC</th>
<th style={{ padding: '10px', textAlign: 'left' }}>Event</th>
<th style={{ padding: '10px', textAlign: 'left' }}>Prediction (p)</th>
<th scope="col" style={{ padding: '10px', textAlign: 'left' }}>Log ID</th>
<th scope="col" style={{ padding: '10px', textAlign: 'left' }}>UTC</th>
<th scope="col" style={{ padding: '10px', textAlign: 'left' }}>Event</th>
<th scope="col" style={{ padding: '10px', textAlign: 'left' }}>Prediction (p)</th>
Comment on lines +74 to +77
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

Adding scope="col" to table headers is a great accessibility improvement. This helps screen readers understand the structure of the table.

</tr>
</thead>
<tbody>
{telemetryData.map((entry) => (
<tr key={entry.logId} style={{ borderBottom: '1px solid #ddd' }}>
<td style={{ padding: '10px' }}>{entry.logId}</td>
<td scope="row" style={{ padding: '10px' }}>{entry.logId}</td>
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

Adding scope="row" to the Log ID column is also a good accessibility practice, ensuring screen readers correctly associate the row header with its data cells.

Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The scope="row" attribute is incorrectly applied to a table data cell (td). The scope attribute is only valid on table header cells (th elements). For row headers, you should use a th element with scope="row" instead of a td element. This is important for screen readers to properly associate the row header with the data cells in the row.

Suggested change
<td scope="row" style={{ padding: '10px' }}>{entry.logId}</td>
<th scope="row" style={{ padding: '10px' }}>{entry.logId}</th>

Copilot uses AI. Check for mistakes.
<td style={{ padding: '10px' }}>{entry.utc}</td>
<td style={{ padding: '10px' }}>{entry.event}</td>
<td style={{ padding: '10px' }}>{entry.prediction.p}</td>
Expand Down
2 changes: 2 additions & 0 deletions data/telemetry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,7 @@ export function getProbabilityData() {
return telemetryData.map(entry => ({
time: new Date(entry.utc).getTime(), // For x-axis
probability: entry.prediction.p,
event: entry.event,
mode: entry.prediction.mode,
Comment on lines +59 to +60
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

Adding event and mode to the getProbabilityData return object is a good change, as it makes this data available for the custom tooltip, enhancing the chart's interactivity and information display.

}));
}
Loading