KPMP 6796 create checkboxes - #76
Conversation
WalkthroughBulkUpload now renders three selectable upload options in an AG Grid table. Checkbox changes update component state and row data. A Submit button displays the selected options or a no-options message. New styles define responsive and Material-themed grid presentation. ChangesBulk upload options
Sequence Diagram(s)sequenceDiagram
participant User
participant AGGrid
participant BulkUpload
participant BrowserAlert
User->>AGGrid: change an option checkbox
AGGrid->>BulkUpload: call handleOptionChange(row, checked)
BulkUpload->>BulkUpload: update selection state and row property
User->>BulkUpload: click Submit
BulkUpload->>BrowserAlert: display selected option names or no-options message
Merge Risk: 🔵 Low · up to The PR adds checkbox-related grid styling, but a Sass lint issue may fail checks, the narrow-screen rule may not hide the intended grid content, and the expected Material appearance may not be applied. The change is mergeable with explicit owner follow-up on these bounded issues. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 353cdee1-92a2-4c4b-93d8-e529c94a5774
📒 Files selected for processing (3)
package.jsonsrc/components/Upload/BulkUpload.jssrc/index.scss
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| <div className="ag-theme-material img-fluid"> | ||
| <AgGridReact | ||
| rowData={this.state.rowData} | ||
| columnDefs={this.getColumns()} | ||
| domLayout='autoHeight' | ||
| onGridReady={this.onGridReady} | ||
| autoSizeStrategy={{type: 'fitGridWidth'}} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- package references ---'
rg -n '"(ag-grid|`@ag-grid`|ag-grid-react)|ag-theme-material|theme(Material|Quartz)?' package.json package-lock.json yarn.lock pnpm-lock.yaml src 2>/dev/null | head -n 240
printf '%s\n' '--- target file ---'
cat -n src/components/Upload/BulkUpload.js | sed -n '1,140p'Repository: KPMP/libra-web
Length of output: 5705
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- index.scss ---'
cat -n src/index.scss | sed -n '1,100p'
printf '%s\n' '--- stylesheet imports ---'
rg -n 'index\.scss|ag-grid.*css|ag-theme|theme:' src package.json
printf '%s\n' '--- exact resolved versions ---'
rg -n '"(ag-grid-community|ag-grid-react)":' package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null | head -n 80Repository: KPMP/libra-web
Length of output: 3642
🌐 Web query:
AG Grid 36.1.0 React theming migration legacy CSS ag-theme-material theme prop default Quartz
💡 Result:
In AG Grid 36.1.0, the recommended approach for theming is the Theming API, which replaces the legacy CSS-based approach [1][2]. The Quartz theme is the current default built-in theme [3][4]. Migration Summary The transition involves moving from importing CSS files and applying classes (e.g., ag-theme-material) to passing a JavaScript theme object via the theme grid option [1][2]. 1. Legacy CSS Support: While legacy themes are deprecated, they remain supported for the foreseeable future [1][5]. You can opt to continue using them by setting the theme grid option to 'legacy' [1][5]. This requires maintaining your existing CSS imports and class-based styling [1]. 2. Adopting the Theming API: To migrate away from legacy CSS, you must: - Remove legacy CSS imports from your application [1][5]. - Remove any manually applied theme classes (like ag-theme-material) from your grid container [1][5]. - Import the desired theme (e.g., themeQuartz or themeMaterial) from the @ag-grid-community/theming package [1][2]. - Pass the imported theme object to the theme grid option [1][2]. 3. Customization: The Theming API allows for extensive customization via the withParams method, which provides TypeScript validation for theme parameters [1][2]. This replaces the need to target CSS classes with custom variables [1][2]. For users transitioning from the Material theme, the Theming API includes a Material-compatible configuration, which can be further customized as needed [4]. The Theming API and legacy CSS methods cannot be used simultaneously on the same page [2]. If you are not yet ready to migrate, setting theme={'legacy'} on your AgGridReact component allows the grid to function using your existing CSS-based styling [1][5].
Citations:
- 1: https://www.ag-grid.com/react-data-grid/theming-migration/
- 2: https://blog.ag-grid.com/introducing-our-new-theming-api/
- 3: https://www.ag-grid.com/javascript-data-grid/themes/
- 4: https://www.ag-grid.com/react-data-grid/themes/
- 5: https://blog.ag-grid.com/upgrading-to-ag-grid-33/
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- tracked AG Grid references ---'
git ls-files | xargs -r rg -n 'ag-grid|ag-theme-material|themeMaterial|themeQuartz|theme=' 2>/dev/null | head -n 240
printf '%s\n' '--- dependency lock files ---'
git ls-files | rg '(^|/)(package-lock\.json|yarn\.lock|pnpm-lock\.yaml)$' || true
printf '%s\n' '--- entrypoint stylesheet references ---'
git ls-files '*.{js,jsx,ts,tsx,scss,css}' | xargs -r rg -n 'index\.scss|index\.css|\.scss' 2>/dev/null | head -n 160Repository: KPMP/libra-web
Length of output: 720
🌐 Web query:
site:ag-grid.com AG Grid 36.1.0 themeMaterial import ag-grid-community theme legacy
💡 Result:
In AG Grid 36.1.0, the "Material" theme and other legacy themes are still supported via the legacy theming system, but they are deprecated and slated for removal in a future major version [1][2][3]. To use the legacy Material theme in version 36.1.0, you have two primary options: 1. Use the "legacy" theme option: You can opt back into the v32 style of themes by setting the theme property in your grid options to "legacy" [1][2][3]. This allows you to continue using the traditional CSS-based theming approach [1][2]. 2. Legacy CSS Import: Legacy themes are applied by importing the corresponding CSS file and adding the appropriate CSS class name to your grid container (e.g., class="ag-theme-material") [1][2][4]. Ensure you include both the structural styles (ag-grid.css) and the theme-specific CSS (ag-theme-material.css) from the ag-grid-community/styles directory [4]. Recommendation: Because legacy themes are deprecated, it is highly recommended to migrate to the new Theming API introduced in version 33 [1][2]. The new API uses JavaScript objects instead of CSS classes to configure themes, providing better type safety and more flexibility with composable themes [1][2][3]. Documentation for this migration is available on the official AG Grid website under the "Migrating to the Theming API" section [1][2][3].
Citations:
- 1: https://www.ag-grid.com/javascript-data-grid/theming-migration/
- 2: https://www.ag-grid.com/react-data-grid/theming-migration/
- 3: https://www.ag-grid.com/angular-data-grid/theming-migration/
- 4: https://www.ag-grid.com/javascript-data-grid/theming-v32-themes/
Opt into the Material theme.
If this grid must use the legacy Material theme, set theme="legacy" on AgGridReact and import both ag-grid.css and ag-theme-material.css. The ag-theme-material class and local overrides do not activate the Material theme under AG Grid 36’s default Theming API, which uses Quartz.
| #uploadOptionsTable { | ||
| @media (max-width: 1200px) { | ||
| .MuiTableCell-alignLeft:not(:first-of-type) { | ||
| display: none !important; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Target AG Grid cells in the responsive rule.
.MuiTableCell-alignLeft is not rendered by AgGridReact. Below 1200px, this rule hides no grid content. Target the relevant .ag-header-cell and .ag-cell selectors, such as the description column by its col-id, so the grid remains usable on narrow screens.
Proposed fix
`#uploadOptionsTable` {
`@media` (max-width: 1200px) {
- .MuiTableCell-alignLeft:not(:first-of-type) {
+ .ag-header-cell[col-id='description'],
+ .ag-cell[col-id='description'] {
display: none !important;
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #uploadOptionsTable { | |
| @media (max-width: 1200px) { | |
| .MuiTableCell-alignLeft:not(:first-of-type) { | |
| display: none !important; | |
| } | |
| } | |
| #uploadOptionsTable { | |
| @media (max-width: 1200px) { | |
| .ag-header-cell[col-id='description'], | |
| .ag-cell[col-id='description'] { | |
| display: none !important; | |
| } | |
| } |
| .ag-theme-material { | ||
|
|
||
| $odd-cell-background-color: #cee5ff; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the blank line before the Sass variable.
Stylelint reports scss/dollar-variable-empty-line-before at Line 36. This can fail the stylesheet lint job.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 36-36: Unexpected empty line before $-variable (scss/dollar-variable-empty-line-before)
(scss/dollar-variable-empty-line-before)
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit
New Features
Style