feat: Debugger Diagnostics C++ driven displays and data - #393
Conversation
| tickFormat: formatYAxisTick, | ||
| }, | ||
| marks: [ | ||
| Plot.differenceY(data, { x: 'time', y: 'value' }), |
There was a problem hiding this comment.
This fixes an issue where the x & y values displayed when hovering the graph (showing where your cursor is pointing at), were not using the same formatting method that the ticks/edge labels were using.
There was a problem hiding this comment.
Here's the new hover tip UI
frgarc
left a comment
frgarc
left a comment
There was a problem hiding this comment.
Looks good to me,
Only comment that concerns me is under App.tsx to avoid flickering as now everything is dynamic.
| handler(eventMessage); | ||
| return true; | ||
| } | ||
| console.warn(`No handler found for incoming event type: ${eventMessage.type}`); |
There was a problem hiding this comment.
I'm thinking that this extension needs a custom output, to show all here.
Not for these changes.
| color: '#ffffff', | ||
| background: '#333333', |
There was a problem hiding this comment.
It is better to use the theming CSS variables: https://code.visualstudio.com/api/references/theme-color
That will handle the case of swapping themes from the UI.
| // Merge schema tabs into the prefab list. Schema tabs whose name matches a prefab replace it; | ||
| // new names are appended. Falls back to all prefabs when no schema has arrived yet. | ||
| const mergedTabs: MergedTab[] = sortedTabPrefabs.map(tab => ({ | ||
| kind: 'prefab' as const, | ||
| name: tab.name, | ||
| tab, | ||
| })); | ||
| for (const descriptor of schema) { | ||
| const existingIndex = mergedTabs.findIndex(t => t.name === descriptor.name); | ||
| if (existingIndex !== -1) { | ||
| mergedTabs[existingIndex] = { kind: 'dynamic' as const, name: descriptor.name, descriptor }; | ||
| } else { | ||
| mergedTabs.push({ kind: 'dynamic' as const, name: descriptor.name, descriptor }); | ||
| } | ||
| } | ||
| mergedTabs.sort((a, b) => a.name.localeCompare(b.name)); | ||
|
|
There was a problem hiding this comment.
This should be under a hook, in this case useMemo.
That will avoid unnecessary re-renders when consuming mergedTabs.
React will detect changes on that only if the dependencies to the hook change.
frgarc
left a comment
frgarc
left a comment
There was a problem hiding this comment.
Thanks for looking over feedback 
|
|
||
| function formatRelativeTime(latestTime: number, tick: number): string { | ||
| const diff = latestTime - tick; | ||
| if (diff < 20) { |
There was a problem hiding this comment.
It would be a good idea to define a constant for this value.
3 participants
Uh oh!
There was an error while loading. Please reload this page.