[FEATURE] Biospecimen data visualization study and tranche filters#456
[FEATURE] Biospecimen data visualization study and tranche filters#456
Conversation
Uncomments the download results button in both timewiseTable.jsx and trainingResultsTable.jsx, making the feature available again after previously being disabled due to a bug.
Changed the HTML document title from 'Biospecimen Summary' to 'Biospecimen Lookup' for consistency with the page's purpose.
Introduces a new 'study' filter group to the BiospecimenFilters component, including its state management, configuration, and prop type validation. This allows users to filter biospecimens by study.
Introduced a new 'study' filter and corresponding options in DEFAULT_FILTERS and FILTER_OPTIONS to support filtering by study type in clinical biospecimen data exploration.
Introduces filtering by study using the centralized getStudyName utility in useFilteredBiospecimenData. Records with unmapped or null study codes are retained, while only those with matching study names are included when the filter is active.
Introduces utility functions and mappings for converting between study codes and display names, as well as helpers for study group categorization and API filter value mapping. These utilities support consistent study code handling across biospecimen data visualization components.
Included the 'study' filter in the resetFilters method to ensure it resets along with other default filters.
Introduces a new 'Study' column to the interactive biospecimen chart table and includes it in the CSV export. Utilizes the getStudyName utility to display the study name for each sample.
Introduces a new bar chart displaying participant counts by study group in the BiospecimenChart component. This includes logic for aggregating participants by study, calculating fixed maximums for consistent chart scaling, and handling bar click events to view samples by study.
Introduced a subtitle to the BiospecimenChart component to instruct users to click bars for sample details. The subtitle is styled for clarity and better user guidance.
Introduces a tranche filter to the biospecimen clinical filters, constants, and filtering logic. Updates filter options, default values, prop types, and the filtering hook to support filtering by tranche using a centralized utility.
Introduces a pie chart displaying participant counts by tranche, using consistent color coding and interactive click events to filter samples. Updates demographic calculations to include tranche data and integrates the new chart into the UI for enhanced data exploration.
Renamed 'Tranche 0' to 'Tranche 0 (PreCOVID)' across chart components, filter options, and data transformation utilities for improved clarity regarding the pre-COVID cohort.
Extended tranche-related constants, filter options, and color assignments to include 'Tranche 5'. Updated the tranche code-to-name mapping and chart color array to ensure consistent handling and display of the new tranche group.
Refactored the click event for chart bars to pass a structured point object with category, sample count, samples, and demographicType. This improves the data provided to the onBarClick callback and removes unused properties.
Increased the chart height calculation and adjusted card container classes to improve layout and visual consistency in the BiospecimenChart component.
Introduces a new tranche group 'Not yet shipped to CAS' to better categorize biospecimens that have not been shipped. Updates the tranche group arrays, color assignments, filter options, and the transformTrancheCode utility to handle null or undefined tranche codes accordingly.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Study and Tranche end-to-end: utilities and mappings, client-side filtering/enrichment, CSV/table exports, Study bar and Tranche pie charts with per-point sample arrays and click handlers, filters and defaults, tranche name updates, and reviewer-agreement gating for reviewer downloads. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Summary of ChangesHello @jimmyzhen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the biospecimen data exploration interface by incorporating 'study' and 'tranche' as core demographic filters and visualization elements. Users can now interactively explore participant distributions across these new dimensions through dedicated charts, and filter the underlying data accordingly. The changes also ensure that these new data points are reflected in the detailed sample tables and CSV exports, providing a more complete and user-friendly data analysis experience. Additionally, a minor bug preventing file downloads in other tables has been resolved. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively adds 'study' and 'tranche' as new demographic dimensions to the biospecimen data exploration UI, introducing new charts, filters, and updating the data table and export functionality. The changes are well-structured and largely follow existing patterns. My review focuses on improving maintainability and performance by suggesting the removal of duplicated constants and hardcoded values, addressing a potential performance bottleneck in data processing logic, and ensuring consistency in filter definitions.
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Outdated
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Outdated
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Outdated
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js
Outdated
Show resolved
Hide resolved
Removed specific file exclusions for timewiseTable.jsx and trainingResultsTable.jsx from the FILTER_REGEX_EXCLUDE pattern in the linter GitHub Actions workflow.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/DataExploration/Biospecimen/Clinical/utils/studyUtils.js (1)
9-34: Consider deriving reverse mapping and groups from the primary mapping.Maintaining three separate data structures (
STUDY_CODE_MAPPING,STUDY_NAME_TO_CODE,STUDY_GROUPS) creates sync risk. Derive them from a single source:🔎 Suggested refactor
export const STUDY_CODE_MAPPING = { '01': 'Adult Sedentary', '02': 'Adult Highly Active', '03': 'Pediatric Low Active', '04': 'Pediatric High Active', }; -export const STUDY_NAME_TO_CODE = { - 'Adult Sedentary': '01', - 'Adult Highly Active': '02', - 'Pediatric Low Active': '03', - 'Pediatric High Active': '04', -}; +// Derived reverse mapping +export const STUDY_NAME_TO_CODE = Object.fromEntries( + Object.entries(STUDY_CODE_MAPPING).map(([code, name]) => [name, code]) +); -export const STUDY_GROUPS = [ - 'Adult Sedentary', - 'Adult Highly Active', - 'Pediatric Low Active', - 'Pediatric High Active', -]; +// Derived groups array +export const STUDY_GROUPS = Object.values(STUDY_CODE_MAPPING);src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (2)
28-29: Consider importing TRANCHE_GROUPS from dataTransformUtils instead of duplicating.
TRANCHE_GROUPSduplicates the values fromTRANCHE_CODE_TO_NAMEindataTransformUtils.js. If a new tranche is added, both files need updates.🔎 Suggested refactor
In
dataTransformUtils.js, add:export const TRANCHE_GROUPS = [...Object.values(TRANCHE_CODE_TO_NAME), 'Not yet shipped to CAS'];Then in this file:
-import { transformTrancheCode } from '../utils/dataTransformUtils'; +import { transformTrancheCode, TRANCHE_GROUPS } from '../utils/dataTransformUtils'; -// Tranche groups for consistent categorization -const TRANCHE_GROUPS = ['Tranche 0 (PreCOVID)', 'Tranche 1', 'Tranche 2', 'Tranche 3', 'Tranche 4', 'Tranche 5', 'Not yet shipped to CAS'];
460-462: Unnecessary O(n²) check — other demographic computations don't dedupe samples.The
includes()check creates O(n²) complexity. Other similar functions likeparticipantByBMIsimply push records without checking:🔎 Suggested fix
// Collect samples for this tranche - if (!samplesByTranche[trancheName].includes(record)) { - samplesByTranche[trancheName].push(record); - } + samplesByTranche[trancheName].push(record);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx(9 hunks)src/DataExploration/Biospecimen/Clinical/components/BiospecimenFilters.jsx(4 hunks)src/DataExploration/Biospecimen/Clinical/components/InteractiveBiospecimenChart.jsx(5 hunks)src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js(3 hunks)src/DataExploration/Biospecimen/Clinical/hooks/useBiospecimenData.js(2 hunks)src/DataExploration/Biospecimen/Clinical/summaryStatistics.jsx(1 hunks)src/DataExploration/Biospecimen/Clinical/utils/dataTransformUtils.js(2 hunks)src/DataExploration/Biospecimen/Clinical/utils/studyUtils.js(1 hunks)src/Search/timewiseTable.jsx(0 hunks)src/Search/trainingResultsTable.jsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/Search/trainingResultsTable.jsx
- src/Search/timewiseTable.jsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/DataExploration/Biospecimen/Clinical/constants/plotOptions.jssrc/DataExploration/Biospecimen/Clinical/utils/studyUtils.jssrc/DataExploration/Biospecimen/Clinical/components/BiospecimenFilters.jsxsrc/DataExploration/Biospecimen/Clinical/components/InteractiveBiospecimenChart.jsxsrc/DataExploration/Biospecimen/Clinical/hooks/useBiospecimenData.jssrc/DataExploration/Biospecimen/Clinical/summaryStatistics.jsxsrc/DataExploration/Biospecimen/Clinical/utils/dataTransformUtils.jssrc/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (21)
src/DataExploration/Biospecimen/Clinical/summaryStatistics.jsx (1)
21-21: LGTM!Title now matches the UI heading "Biospecimen Lookup" — consistent naming across SEO and UI.
src/DataExploration/Biospecimen/Clinical/components/InteractiveBiospecimenChart.jsx (3)
15-15: LGTM!Import is correctly added to support study name resolution in exports and table display.
93-93: LGTM!CSV export correctly includes the Study column with header and data row in sync.
Also applies to: 108-108
276-276: LGTM!Table column correctly displays study name with 'N/A' fallback for missing data.
Also applies to: 300-300
src/DataExploration/Biospecimen/Clinical/hooks/useBiospecimenData.js (3)
11-12: LGTM!Imports for study and tranche utilities support the new client-side filtering logic below.
218-226: LGTM!Study filter correctly preserves records with missing/unknown study codes while filtering records with mapped study names.
228-236: Verify tranche filter behavior for null values.Note that
transformTrancheCode(null)returns'Not yet shipped to CAS'(a truthy value), so records with null tranche codes will be filtered out if that option isn't selected. This differs from other filters that preserve null records. Verify this is the intended behavior.If records with null tranche should be preserved regardless of filter selection (like other demographic filters), consider:
🔎 Suggested fix to preserve null tranche records
// Filter by tranche - using centralized utility // Only filter out if record HAS a tranche code that doesn't match selected filters // Records with null/unmapped tranche codes are kept (don't filter based on missing data) if (filters.tranche?.length > 0) { + // Skip filtering for records without tranche data + if (!item.tranche) return true; const trancheName = transformTrancheCode(item.tranche); if (trancheName && !filters.tranche.includes(trancheName)) { return false; } }src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js (3)
64-66: Potential mismatch between default tranche filter and available options.
DEFAULT_FILTERS.tranchedoesn't include'Not yet shipped to CAS', buttrancheOptionsdoes. Combined withtransformTrancheCode(null)returning'Not yet shipped to CAS', this means records with null tranche will be filtered out by default.If you want to show all records by default (including those not yet shipped to CAS), add the option:
🔎 Suggested fix
- tranche: ['Tranche 0 (PreCOVID)', 'Tranche 1', 'Tranche 2', 'Tranche 3', 'Tranche 4', 'Tranche 5'], // All selected by default + tranche: ['Tranche 0 (PreCOVID)', 'Tranche 1', 'Tranche 2', 'Tranche 3', 'Tranche 4', 'Tranche 5', 'Not yet shipped to CAS'], // All selected by default
85-86: LGTM!Filter options are consistent with the utility mappings and include all expected values.
133-134: LGTM!Reset logic correctly includes the new study and tranche filters.
src/DataExploration/Biospecimen/Clinical/utils/dataTransformUtils.js (2)
12-12: LGTM!Tranche mappings updated with clearer TR00 label and new TR05 support.
Also applies to: 17-17
36-39: LGTM!Null handling provides meaningful display for records without tranche data. Note the filtering implications flagged in
useBiospecimenData.js.src/DataExploration/Biospecimen/Clinical/utils/studyUtils.js (1)
41-67: LGTM!Utility functions are clean and handle edge cases appropriately.
mapStudyFiltersToAPIValuescorrectly filters invalid entries.src/DataExploration/Biospecimen/Clinical/components/BiospecimenFilters.jsx (3)
27-28: LGTM!Expanded groups state correctly initialized for new filter sections.
89-100: LGTM!Filter configurations follow the established data-driven pattern. Clean and consistent.
183-184: LGTM!PropTypes correctly extended to include the new study and tranche filter requirements.
Also applies to: 195-196
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (5)
90-142: LGTM!Fixed max calculations efficiently extended in a single pass. Pattern is consistent with existing demographic counts.
399-439: LGTM!
participantByStudyfollows the established pattern with proper sample attachment for drill-down.
988-1065: LGTM!Study bar chart configuration follows established patterns with proper drill-down support.
1067-1119: LGTM!Tranche pie chart configuration is consistent with other pie charts and includes proper click handling.
1393-1422: LGTM!New chart row follows the existing layout pattern with proper fallback for missing data.
There was a problem hiding this comment.
Pull request overview
This pull request adds study and tranche as new demographic dimensions for filtering and visualizing biospecimen data. The changes introduce new chart visualizations, expand filter capabilities, and enhance the table display to include study information.
Key changes:
- Added study and tranche as filterable demographic dimensions with new bar and pie chart visualizations
- Re-enabled download functionality in training and timewise result tables
- Updated page title from "Biospecimen Summary" to "Biospecimen Lookup"
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Search/trainingResultsTable.jsx | Re-enabled commented-out download button functionality |
| src/Search/timewiseTable.jsx | Re-enabled commented-out download button functionality |
| src/DataExploration/Biospecimen/Clinical/utils/studyUtils.js | New utility file for study code mapping and transformation |
| src/DataExploration/Biospecimen/Clinical/utils/dataTransformUtils.js | Added TR05 tranche mapping and updated TR00 label; changed null tranche handling to return 'Not yet shipped to CAS' |
| src/DataExploration/Biospecimen/Clinical/summaryStatistics.jsx | Updated page title from "Biospecimen Summary" to "Biospecimen Lookup" |
| src/DataExploration/Biospecimen/Clinical/hooks/useBiospecimenData.js | Added study and tranche filtering logic with imports for new utility functions |
| src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js | Added study and tranche to default filters and filter options |
| src/DataExploration/Biospecimen/Clinical/components/InteractiveBiospecimenChart.jsx | Added Study column to sample table and CSV export |
| src/DataExploration/Biospecimen/Clinical/components/BiospecimenFilters.jsx | Added study and tranche filter UI components and prop types |
| src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx | Added study bar chart and tranche pie chart visualizations; updated chart sizing and styling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Outdated
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Outdated
Show resolved
Hide resolved
src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js
Outdated
Show resolved
Hide resolved
Replaces the hardcoded TRANCHE_GROUPS array with FILTER_OPTIONS.trancheOptions for consistent tranche categorization. This change improves maintainability by centralizing tranche options in the constants file.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (5)
91-91: Remove unused variablefixedMaxTrancheCount.This variable is calculated but never used. The tranche pie chart doesn't require a fixed maximum scale (pie charts display percentages).
461-463: Remove or optimize the inefficient duplicate check.The
includes()check results in O(n²) complexity. This is inconsistent withparticipantByStudyand other demographic calculations that don't perform this check. If duplicate prevention is critical, use aSetof sample IDs; otherwise, remove this check.
477-477: Centralize tranche colors for better maintainability.The hardcoded colors array makes the component less maintainable. Define these colors in
plotOptions.jsorcolors.jsand import them here.
1040-1040: Centralize the hardcoded color.Define
'#16a085'as a named constant inplotOptions.jsorcolors.jsfor better maintainability and consistency.
1130-1130: Extract magic numbers to named constants.The values
600,45, and120should be extracted into descriptive constants likeMIN_CHART_HEIGHT,BAR_HEIGHT_PX, andCHART_VERTICAL_PADDINGat the top of the file.
🧹 Nitpick comments (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (1)
989-1066: Study chart implementation is solid, but centralize the color.The chart configuration follows established patterns and correctly implements drill-down functionality. However, the hardcoded color
'#16a085'at line 1040 should be defined in a centralized location likeplotOptions.jsorcolors.js.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/linter.ymlis excluded by!**/*.yml
📒 Files selected for processing (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
🧬 Code graph analysis (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (3)
src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js (2)
FILTER_OPTIONS(76-87)FILTER_OPTIONS(76-87)src/DataExploration/Biospecimen/Clinical/utils/dataTransformUtils.js (2)
transformTrancheCode(36-40)transformTrancheCode(36-40)src/lib/colors.js (1)
colors(1-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (5)
12-12: LGTM! Clean imports and constant definition.The study and tranche utilities are properly imported, and
TRANCHE_GROUPSis correctly sourced fromFILTER_OPTIONS.trancheOptionsas a single source of truth.Also applies to: 26-27, 29-30
400-440: LGTM! Study participant calculation follows established patterns.The implementation is consistent with other demographic calculations, properly tracks unique participants, and attaches samples for drill-down functionality.
1068-1120: LGTM! Tranche pie chart follows established patterns.The configuration is consistent with other pie chart implementations and correctly implements interactive drill-down functionality.
1138-1142: LGTM! Helpful subtitle addition.The subtitle provides clear user guidance about chart interactivity.
1287-1287: LGTM! Clean chart rendering implementation.The study and tranche charts are properly integrated with consistent layout, null checks, and fallback messaging. The d-flex flex-column classes ensure proper card height handling.
Also applies to: 1394-1423, 1428-1428
Removes the 'reviewerAgreement' item from sessionStorage when the user logs out to ensure session data is properly cleared.
Revised the contact summary to direct users to the Assistance Request form, web form, or helpdesk email for inquiries. Updated the text for clarity and added a link to the external form.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Dashboard/dashboard.jsx (1)
273-279: Missingapp_metadatain PropTypes.The component accesses
profile.app_metadata.roleat line 36, butapp_metadatais not defined in the PropTypes shape forprofile.🔎 Proposed fix
Dashboard.propTypes = { profile: PropTypes.shape({ user_metadata: PropTypes.object, + app_metadata: PropTypes.shape({ + role: PropTypes.string, + }), }), handleQCDataFetch: PropTypes.func.isRequired, lastModified: PropTypes.string, };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/ContactPage/contact.jsx(1 hunks)src/Dashboard/dashboard.jsx(4 hunks)src/Dashboard/reviewerDownloadButton.jsx(3 hunks)src/Navbar/navbar.jsx(1 hunks)src/sass/dashboard.scss(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/sass/dashboard.scss
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/Dashboard/reviewerDownloadButton.jsxsrc/ContactPage/contact.jsxsrc/Navbar/navbar.jsxsrc/Dashboard/dashboard.jsx
🪛 Biome (2.1.2)
src/ContactPage/contact.jsx
[error] 31-31: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
src/Navbar/navbar.jsx (1)
101-110: LGTM! Proper session cleanup on logout.Clearing the reviewer agreement from sessionStorage ensures no stale agreement state persists across sessions. This correctly coordinates with the agreement flow in the Dashboard component.
src/Dashboard/reviewerDownloadButton.jsx (1)
24-30: LGTM! Clean implementation of the disabled prop.The
disabledprop is properly typed, has a sensible default, and is correctly wired to the button element. This enables external control of the download button state for the agreement flow.Also applies to: 153-163, 166-176
src/Dashboard/dashboard.jsx (3)
30-33: LGTM! Proper sessionStorage initialization.Using a function initializer with
useStateensures the sessionStorage read happens only once on mount, not on every render. The string comparisonsaved === 'true'correctly handles the string nature of sessionStorage values.
53-62: LGTM! Agreement handlers correctly persist state.Both handlers properly update React state and sessionStorage in sync. The explicit 'false' value in
handleCancelensures the user's choice is tracked, though it has the same practical effect as leaving it undefined.
196-256: Modal accessibility and structure look solid.The modal uses
data-backdrop="static"anddata-keyboard="false"to prevent dismissal without explicit agreement, which aligns with the intended UX. The semantic structure with clear agreement terms and explicit I agree/I disagree actions is well-implemented.
Added a note about dependencies among the R packages and recommended installing all four. Reordered the download buttons to list Function, Data, and Analysis R Packages in alignment with the new guidance.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Dashboard/dashboard.jsx (1)
39-50: Memory leak: Bootstrap Modal instance is never disposed.This issue was previously flagged but remains unresolved. The modal instance created inside the dynamic import promise is never cleaned up when the component unmounts or dependencies change. Additionally, if the component unmounts before the import completes, accessing
modalRef.currentcould fail.🔎 Recommended fix with proper cleanup
useEffect(() => { + let modalInstance = null; + let isMounted = true; + if (userType === 'external' && userRole === 'reviewer' && !agreement && modalRef.current) { - // Use Bootstrap's native JavaScript Modal API (no jQuery required) import('bootstrap').then((bootstrap) => { + if (!isMounted || !modalRef.current) return; const modalInstance = new bootstrap.Modal(modalRef.current, { backdrop: 'static', keyboard: false }); modalInstance.show(); - }); + }).catch((err) => { + console.error('Failed to load Bootstrap modal:', err); + }); } + + return () => { + isMounted = false; + if (modalInstance) { + modalInstance.dispose(); + } + }; }, [userType, userRole, agreement]);
🧹 Nitpick comments (1)
src/Dashboard/dashboard.jsx (1)
29-33: Add error handling for sessionStorage access.sessionStorage operations can throw exceptions in restricted environments (e.g., private browsing mode, disabled storage). Consider wrapping sessionStorage calls in try-catch blocks for defensive coding.
🔎 Proposed defensive sessionStorage access
- const [agreement, setAgreement] = useState(() => { - const saved = sessionStorage.getItem('reviewerAgreement'); - return saved === 'true'; - }); + const [agreement, setAgreement] = useState(() => { + try { + const saved = sessionStorage.getItem('reviewerAgreement'); + return saved === 'true'; + } catch (e) { + console.warn('sessionStorage not available:', e); + return false; + } + });const handleAgree = () => { setAgreement(true); - sessionStorage.setItem('reviewerAgreement', 'true'); + try { + sessionStorage.setItem('reviewerAgreement', 'true'); + } catch (e) { + console.warn('Failed to persist agreement:', e); + } };const handleCancel = () => { setAgreement(false); - sessionStorage.setItem('reviewerAgreement', 'false'); + try { + sessionStorage.setItem('reviewerAgreement', 'false'); + } catch (e) { + console.warn('Failed to persist disagreement:', e); + } };Also applies to: 55-55, 61-61
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Dashboard/dashboard.jsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/Dashboard/dashboard.jsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
src/Dashboard/dashboard.jsx (1)
151-263: LGTM!The reviewer agreement flow is well structured. The modal content clearly communicates data use terms, and the conditional disabling of download buttons based on agreement state provides appropriate access control.
Refactored logic to ensure participants are counted uniquely per tranche, addressing issues with duplicate counting across tranches. Updated data structures for clarity and correctness in the pie chart data.
Added rel="noopener noreferrer" to the Assistance Request form link to improve security when opening the link in a new tab.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (1)
95-147: Fix inconsistent participant counting for tranche demographics.The calculation stores each participant with a single tranche value (line 110), but
participantByTranche(lines 447-486) counts participants per tranche separately. This inconsistency means:
- If a participant has samples in multiple tranches,
fixedMaxTrancheCountonly counts them once (in their first tranche encountered).- The tranche pie chart counts them in each tranche they appear in.
- This causes
fixedMaxTrancheCountto potentially underestimate, though it's not used for the pie chart.Since tranches represent sequential data collection phases, participants legitimately span multiple tranches. The counting logic should be consistent: either single-tranche-per-participant everywhere, or per-tranche counting everywhere.
Note:
fixedMaxTrancheCountis calculated but appears unused (no chart uses it for y-axis max). If it's intended for future bar charts of tranche data, it should follow the per-tranche counting pattern.🔎 Proposed fix to align counting strategies
If
fixedMaxTrancheCountwill be used for a bar chart in the future, update the logic to count per tranche:- const trancheCounts = {}; - TRANCHE_GROUPS.forEach(group => trancheCounts[group] = 0); + // Track participants per tranche (not globally unique) + const participantsByTranche = {}; + TRANCHE_GROUPS.forEach(group => participantsByTranche[group] = new Set()); + + allData.forEach((record) => { + if (record.pid) { + const trancheName = transformTrancheCode(record.tranche); + if (trancheName && TRANCHE_GROUPS.includes(trancheName)) { + participantsByTranche[trancheName].add(record.pid); + } + } + }); + + const trancheCounts = {}; + TRANCHE_GROUPS.forEach(group => { + trancheCounts[group] = participantsByTranche[group].size; + });Then remove the tranche tracking from the participantDemographics map since it's no longer needed for this calculation.
♻️ Duplicate comments (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (1)
447-486: Verify participant counting logic for multi-tranche participants.Lines 458-459 store only the first tranche per participant, but line 463 adds samples to every tranche they appear in. This means:
- A participant with samples in Tranche 1, 2, and 3 is counted once (in Tranche 1 only).
- But their samples appear in all three tranches.
- This creates a mismatch between participant counts and sample counts.
Question: Should participants be counted once globally, or once per tranche?
- If once globally: Move sample collection logic to only track samples for the participant's assigned tranche.
- If once per tranche (current sample behavior): Remove the
!uniqueParticipants.has(record.pid)check so each tranche gets its own count.Given that tranches represent sequential data collection phases, counting per tranche seems more accurate. This matches the comment from graphite-app[bot] that was marked "Resolved per comment."
🔎 Verify the intended counting strategy
If participants should be counted per tranche:
- const uniqueParticipants = new Map(); + const participantsByTranche = {}; + TRANCHE_GROUPS.forEach(group => participantsByTranche[group] = new Set()); + const samplesByTranche = {}; TRANCHE_GROUPS.forEach(group => samplesByTranche[group] = []); data.forEach((record) => { if (record.pid) { const trancheName = transformTrancheCode(record.tranche); if (trancheName) { - if (!uniqueParticipants.has(record.pid)) { - uniqueParticipants.set(record.pid, trancheName); - } - // Guard against unexpected tranche values if (samplesByTranche[trancheName]) { + participantsByTranche[trancheName].add(record.pid); samplesByTranche[trancheName].push(record); } } } }); const trancheCounts = {}; TRANCHE_GROUPS.forEach(group => trancheCounts[group] = 0); - uniqueParticipants.forEach((tranche) => { - if (Object.hasOwn(trancheCounts, tranche)) { - trancheCounts[tranche]++; - } - }); + TRANCHE_GROUPS.forEach(group => { + trancheCounts[group] = participantsByTranche[group].size; + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx(15 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
🧬 Code graph analysis (1)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (2)
src/DataExploration/Biospecimen/Clinical/constants/plotOptions.js (2)
FILTER_OPTIONS(76-87)FILTER_OPTIONS(76-87)src/DataExploration/Biospecimen/Clinical/utils/dataTransformUtils.js (2)
transformTrancheCode(36-40)transformTrancheCode(36-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
🔇 Additional comments (7)
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx (7)
404-444: LGTM! Study demographic calculation follows established patterns.The implementation correctly mirrors the existing demographic calculation logic for age, race, and other dimensions. It properly normalizes study values via
getStudyName, tracks unique participants, and collects samples for drill-down functionality.
991-1068: LGTM! Study chart configuration follows established patterns.The bar chart implementation correctly mirrors existing demographic charts (age, race, randomized group). It uses
fixedMaxStudyCountfor consistent scaling, includes interactive drill-down, and properly formats labels and tooltips.Note: Line 1042 uses a hardcoded color
'#16a085', which was previously flagged and deferred to future updates per past review comments.
1070-1122: LGTM! Tranche pie chart configuration follows established patterns.The pie chart implementation correctly mirrors existing pie charts (sex, BMI, ethnicity). It includes interactive drill-down and properly formats labels and tooltips.
Note: Line 1079 uses hardcoded colors, which was previously flagged and deferred to future updates per past review comments.
1132-1144: LGTM! Chart height calculation and subtitle enhance UX.The dynamic height calculation uses well-named constants (addressing previous magic number concerns), and the subtitle clearly indicates interactive functionality to users.
1289-1430: LGTM! Layout updates maintain consistent structure.The layout additions properly integrate the new study and tranche charts while maintaining the existing grid structure. The flex classes ensure consistent card heights, and proper null checks provide clear fallback messages.
26-27: Add null/undefined guard togetStudyNamefor consistency withtransformTrancheCode.The
transformTrancheCodefunction explicitly handles null/undefined inputs with a meaningful fallback message, butgetStudyNamesilently returns null without an explicit guard. While calling code includes null checks, the utility function should be self-defensive liketransformTrancheCodeto prevent silent failures and ensure consistent error handling patterns across related utilities.
204-204: Object.hasOwn() usage is appropriate for this React 18 codebase.React 18.3.1 targets modern browsers only (IE support was dropped), and Vite's default configuration aligns with this.
Object.hasOwn()is the recommended modern approach and has solid browser support (~88%+). No compatibility concerns here.
Replaces dynamic import of Bootstrap's JS Modal API with jQuery-based modal handling for reviewer agreement. Adds cleanup to hide modal on component unmount.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Dashboard/dashboard.jsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/Dashboard/dashboard.jsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
src/Dashboard/dashboard.jsx (6)
1-1: LGTM! Necessary imports for state and modal management.The addition of
useState,useEffect, anduseRefcorrectly supports the new reviewer agreement flow.
15-15: LGTM! Constant follows existing pattern.The new
PACK_CLINICAL_ANALYSISconstant is consistent with other package constants and properly integrated into the download UI.
29-34: LGTM! Solid state management with sessionStorage persistence.The lazy initialization and sessionStorage integration correctly persist the agreement state across page navigations. The ref setup is appropriate for the modal element.
51-61: LGTM! Clean and symmetric agreement handlers.Both handlers correctly update state and persist to sessionStorage. The separation of concerns (state update in handlers, modal dismissal via data-dismiss attribute) is appropriate.
170-197: LGTM! Proper gating of downloads behind agreement.The
disabled={!agreement}prop on all download buttons correctly enforces the data use agreement flow. The new Clinical Analysis R Package is consistently integrated with the existing three packages.
242-257: LGTM! Bootstrap 4 syntax is correct here.The
data-dismiss="modal"attribute (lines 245, 253) is the correct syntax for Bootstrap 4.6.1, which is the version used in this project (per library context). The past comment from graphite-app[bot] suggestingdata-bs-dismissis incorrect—that syntax only applies to Bootstrap 5.The modal markup is well-structured with proper accessibility attributes and the agreement/dismissal flow works correctly with the onClick handlers.
Note: The past review comment suggesting Bootstrap 5 syntax (
data-bs-dismiss) is incorrect for this codebase, which uses Bootstrap 4.6.1.
src/DataExploration/Biospecimen/Clinical/components/BiospecimenChart.jsx
Show resolved
Hide resolved
Removed useRef for the modal and switched to direct jQuery Bootstrap modal handling. This simplifies the modal display logic for reviewers who haven't agreed yet.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Dashboard/dashboard.jsx (1)
237-252: Bootstrap 4 syntax is correct; verify modal dismissal works.A past review comment incorrectly suggested changing
data-dismiss="modal"todata-bs-dismiss="modal". The current code is correct for Bootstrap 4.6.1 (which this project uses). However, ensure that clicking these buttons actually dismisses the modal, as the handlers only update state and don't programmatically call.modal('hide').Run the following to verify the Bootstrap version and that modal dismissal works as expected:
#!/bin/bash # Check Bootstrap version in package.json echo "Bootstrap version:" cat package.json | jq -r '.dependencies.bootstrap // .devDependencies.bootstrap // "not found"' # Check if there are other modal implementations that might conflict echo -e "\nSearching for other modal usage patterns:" rg -n --type js --type jsx -C2 "\.modal\(|Modal\(|data-dismiss|data-bs-dismiss" src/
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Dashboard/dashboard.jsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: 'Focus on code quality, performance, and security best practices for JavaScript and TypeScript files. Ensure that:
- Code adheres to modern JavaScript/TypeScript standards and ES6+ features.
- The code adheres to best practices associated with React.
- The code adheres to best practices associated with React PWA.
- The code adheres to best practices associated with SPA.
- Proper error handling is implemented.
- Security vulnerabilities such as XSS, CSRF, and injection attacks are mitigated.
- Performance optimizations are considered, especially for large data sets or complex computations.
- Code is modular, reusable, and follows SOLID principles where applicable.
- Type safety is enforced in TypeScript files, with appropriate use of interfaces and types.'
Files:
src/Dashboard/dashboard.jsx
🧬 Code graph analysis (1)
src/Dashboard/dashboard.jsx (4)
src/Navbar/navbar.jsx (2)
userType(122-122)userRole(123-123)src/Search/searchPage.jsx (2)
userType(58-58)userRole(59-59)src/BrowseDataPage/browseDataFilter.jsx (2)
userType(13-13)profile(12-12)src/Home/HomePage.jsx (1)
userType(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
src/Dashboard/dashboard.jsx (3)
31-35: LGTM: Agreement state properly initialized from sessionStorage.The pattern of initializing state from sessionStorage ensures persistence across page navigations while keeping the state reactive. Good use of lazy initialization with the function form of
useState.
46-56: LGTM: Handlers correctly manage agreement state.Both handlers properly update component state and persist to sessionStorage. The
handleCancelexplicitly sets agreement to false, ensuring buttons remain disabled if the user declines.
165-192: LGTM: Download buttons correctly gated by agreement.All four download buttons consistently use the
disabled={!agreement}prop, ensuring users must agree to the terms before downloading. The new Clinical Analysis package is properly integrated alongside existing packages.
Added a cleanup function to the useEffect hook to hide the reviewer agreement modal when the component unmounts or dependencies change. This prevents the modal from persisting unexpectedly.
The 'profile' prop was removed from ClinicalBiospecimenSummaryStatistics as it was not being used. The component now relies solely on the Redux store for user profile data.
Eliminated the import and usage of getAssayFullNames from assayCodeMapping as it is no longer needed in BiospecimenChart.jsx. This cleans up unused code and streamlines the tooltip formatter.
Eliminated the calculation and usage of tranche counts and fixedMaxTrancheCount from the BiospecimenChart component, streamlining the demographic chart logic to only include age, race, random group, and study counts.
Replaces double quotes with HTML entities in the agreement text to ensure proper rendering in JSX.
Added ktype as a required string property to the searchParams prop types in timewiseTable and trainingResultsTable components to ensure proper validation and usage.
Eliminates the 'tranche' property from participantDemographics and related logic in BiospecimenChart.jsx, as it is no longer required for chart rendering or data processing.
Simplified and rounded gradient color values for consistency and readability. Refactored the .btn.reviewer-data-download-link:disabled selector for improved specificity and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/sass/dashboard.scss (3)
22-24: LGTM: Correct disabled button cursor.The
cursor: not-allowed;appropriately signals to users that the reviewer download button is disabled.Optional: Simplify selector if .btn is always present
If the
.btnclass is always applied, consider simplifying to:-.btn.reviewer-data-download-link:disabled { +.reviewer-data-download-link:disabled { cursor: not-allowed; }This reduces specificity and improves maintainability.
9-9: Simplify decimal precision for maintainability.The HSL hue values (e.g.,
295.999,236.912,186.618) and percentages (e.g.,3.121%) use unnecessarily precise decimals. Rounding to whole numbers or one decimal place improves readability without visual impact.Example: Round HSL values
-background-image: radial-gradient(circle at 0% 0%, hsla(295.999, 77%, 74%, 0.35) 3.121%, transparent 40%), radial-gradient(circle at 20% 0%, hsla(236.912, 77%, 74%, 0.35) 3.121%, transparent 40%), radial-gradient(circle at 40% 0%, hsla(186.618, 77%, 74%, 0.35) 3.121%, transparent 40%), radial-gradient(circle at 60% 0%, hsla(127.059, 77%, 74%, 0.35) 3.121%, transparent 40%), radial-gradient(circle at 80% 0%, hsla(62.206, 77%, 74%, 0.35) 3.121%, transparent 40%), radial-gradient(circle at 100% 0%, hsla(23.824, 77%, 74%, 0.35) 3%, transparent 40%); +background-image: radial-gradient(circle at 0% 0%, hsla(296, 77%, 74%, 0.35) 3%, transparent 40%), radial-gradient(circle at 20% 0%, hsla(237, 77%, 74%, 0.35) 3%, transparent 40%), radial-gradient(circle at 40% 0%, hsla(187, 77%, 74%, 0.35) 3%, transparent 40%), radial-gradient(circle at 60% 0%, hsla(127, 77%, 74%, 0.35) 3%, transparent 40%), radial-gradient(circle at 80% 0%, hsla(62, 77%, 74%, 0.35) 3%, transparent 40%), radial-gradient(circle at 100% 0%, hsla(24, 77%, 74%, 0.35) 3%, transparent 40%);
34-35: Simplify decimal precision and consider hover performance.Similar to line 9, the HSL values and percentages use excessive precision. Additionally, five radial-gradients in a hover state may cause repainting overhead on lower-end devices.
Recommendations
- Round decimal values:
-background-color: hsla(54.265, 91%, 97%, 1); -background-image: radial-gradient(circle at 92% 6%, hsla(165.441, 73%, 96%, 0.92) 0%, transparent 58.066%), radial-gradient(circle at 0% 50%, hsla(42.353, 100%, 97%, 1) 0%, transparent 50%), radial-gradient(circle at 83% 89%, hsla(37.059, 88%, 95%, 1) 0%, transparent 78.774%), radial-gradient(circle at 0% 100%, hsla(38.382, 100%, 86%, 1) 0%, transparent 50%), radial-gradient(circle at 0% 0%, hsla(56.912, 100%, 99%, 1) 0%, transparent 50%); +background-color: hsla(54, 91%, 97%, 1); +background-image: radial-gradient(circle at 92% 6%, hsla(165, 73%, 96%, 0.92) 0%, transparent 58%), radial-gradient(circle at 0% 50%, hsla(42, 100%, 97%, 1) 0%, transparent 50%), radial-gradient(circle at 83% 89%, hsla(37, 88%, 95%, 1) 0%, transparent 79%), radial-gradient(circle at 0% 100%, hsla(38, 100%, 86%, 1) 0%, transparent 50%), radial-gradient(circle at 0% 0%, hsla(57, 100%, 99%, 1) 0%, transparent 50%);
- Performance note: If hover performance becomes an issue, consider simpler transitions or
will-change: backgroundfor GPU acceleration.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sass/dashboard.scss(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{css,scss,sass,less}
⚙️ CodeRabbit configuration file
**/*.{css,scss,sass,less}: 'Review the CSS code against the google css style guide and point out any mismatches. Ensure that:
- The code adheres to best practices associated with CSS.
- The code adheres to best practices recommended by lighthouse or similar tools for performance.
- The code adheres to similar naming conventions for classes, ids.'
Files:
src/sass/dashboard.scss
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
Set VALIDATE_CSS to false in the GitHub Actions linter workflow, disabling CSS/SCSS validation. Other validations remain enabled.
Set VALIDATE_CSS to true in the GitHub Actions linter workflow to include CSS/SCSS validation during automated checks.
…alidation Disabled the CSS/SCSS validation step in the GitHub Actions linter workflow. This change may be intended to streamline the workflow or address issues with the CSS validator.
This pull request adds support for study and tranche as new demographic dimensions in the biospecimen data exploration UI. It introduces new visualizations (bar and pie charts) for participant distribution by study and tranche, updates the filters and table views to include these dimensions, and ensures consistent data transformation and display across components.
Major enhancements to biospecimen data exploration:
Support for study and tranche as demographic dimensions:
BiospecimenChart.jsxfor these dimensions. This includes logic to group, count, and display participants by study and tranche, with interactive chart elements for sample exploration.BiospecimenFilters.jsxto allow filtering by study and tranche, updating filter state, rendering, and prop types accordingly.plotOptions.jsto include all study and tranche groups, ensuring consistent filtering and selection.Table and export improvements:
InteractiveBiospecimenChart.jsx, ensuring users can view and download study information for each sample.UI/UX enhancements:
BiospecimenChart.jsx.These changes collectively provide users with more granular and interactive ways to explore biospecimen data by study and tranche, in addition to existing demographic filters.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.