Skip to content

feat: pie chart for Job Monitor#380

Merged
aldbr merged 4 commits intoDIRACGrid:mainfrom
TheauW:twartel-job-chart
Aug 7, 2025
Merged

feat: pie chart for Job Monitor#380
aldbr merged 4 commits intoDIRACGrid:mainfrom
TheauW:twartel-job-chart

Conversation

@TheauW
Copy link
Contributor

@TheauW TheauW commented Jul 16, 2025

I added a button to the JobMonitor to visualize jobs using the Sunburst chart. This will be reused in the Bookkeeping Storage report.

I also exposed the columnSelector component, which will be reused in the Bookkeeping Storage report.

I updated the Sunburst to reflect recent changes suggested in gitlab/lhcbdiracx-web and added the ability to refresh jobs.

@TheauW TheauW force-pushed the twartel-job-chart branch from 2854065 to 1458d25 Compare July 16, 2025 11:06
@TheauW TheauW marked this pull request as draft July 16, 2025 14:41
@TheauW TheauW force-pushed the twartel-job-chart branch from 5adef1f to dd2dd95 Compare July 17, 2025 13:31
@TheauW TheauW marked this pull request as ready for review July 17, 2025 13:31
Copy link
Contributor

@ryuwd ryuwd left a comment

Choose a reason for hiding this comment

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

Looks good with some small suggestions!

@aldbr aldbr requested a review from Copilot July 17, 2025 16:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds pie chart visualization capabilities to the JobMonitor component using a Sunburst chart. The implementation includes a new chart component that allows users to visualize job data hierarchically and adds a toggle to switch between table and chart views.

  • Adds a new Sunburst chart component for hierarchical data visualization
  • Implements a column selector component for configuring chart grouping
  • Adds a toggle button to switch between table and chart views in JobMonitor

Reviewed Changes

Copilot reviewed 42 out of 44 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
packages/diracx-web-components/src/components/JobMonitor/JobMonitor.tsx Added chart type selector and JobSunburst component integration
packages/diracx-web-components/src/components/JobMonitor/JobSunburst.tsx New component for sunburst visualization of job data
packages/diracx-web-components/src/components/shared/Sunburst/Sunburst.tsx Core D3-based sunburst chart implementation
packages/diracx-web-components/src/components/shared/ColumnSelector.tsx New component for selecting and ordering chart columns
packages/diracx-web-components/src/components/shared/ChartDisplayLayout.tsx Layout component for chart and column selector
Comments suppressed due to low confidence (1)

packages/diracx-web-components/src/components/shared/Sunburst/Sunburst.tsx:208

  • The magic number 0.05 (5% threshold) should be extracted to a constant or made configurable for better maintainability.
          mouseOut.call(this, event, p);

@TheauW TheauW force-pushed the twartel-job-chart branch from dd2dd95 to e804c1a Compare July 21, 2025 08:27

if (filters.length !== 0 && tokenEquations.length === 0) load();
}, [filters, createSuggestions]);
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you choose not to include filters, tokenEquations, convertFilterToTokenEquation and setTokenEquations here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally wanted to initialize the tokens only once at startup, but I realized it makes sense to update them whenever the filters change (external changes). I fixed it here: #388.
I didn't do it in this PR because I couldn't just add the dependencies — I did that in a separate one.

@TheauW TheauW force-pushed the twartel-job-chart branch from 37ee3de to d4f0131 Compare August 7, 2025 13:22
@TheauW TheauW force-pushed the twartel-job-chart branch from d4f0131 to cf0fec0 Compare August 7, 2025 13:39
@aldbr aldbr merged commit c589a96 into DIRACGrid:main Aug 7, 2025
14 checks passed
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.

3 participants