Conversation
fix glom schematic
open accordions
add papillary epithelial cell
fix proximal tubules
add citation
WalkthroughThis PR extends the Explorer component's renal schematic visualizations by adding two new cell types to the enumeration, updating cell-type-to-ontology ID mappings, refactoring glomerulus and tubule schematic event handlers to route interactions based on SVG group IDs and cell types, and wiring additional state props to the Vessels tab while adding citations. ChangesRenal Schematic Cell Type Handling and State Wiring
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (5)
src/components/Explorer/GlomerulusSchematic.js (2)
31-31: ⚡ Quick winRemove debugging console.log statements.
These console.log statements appear to be debugging artifacts left in the code. They should be removed before merging to production.
🧹 Proposed fix to remove console.log statements
case "Afferent_Arteriole_Endothelial_Cell": - console.log("Clicked on Afferent Arteriole Endothelial Cell"); setActiveCell(CellTypeEnum.AFFERENT_ARTERY_ARTERIOLE_ENDOTHELIAL_CELL); setActiveTab('4'); break; case "Efferent_Arteriole_Endothelial_Cell": - console.log("Clicked on Efferent Arteriole Endothelial Cell"); setActiveCell(CellTypeEnum.EFFERENT_ARTERIOLE_ENDOTHELIAL_CELL); setActiveTab('4'); break; case "Juxtaglomerular_Granular_Cell": - console.log("Clicked on Juxtaglomerular_Granular_Cell"); setActiveCell(CellTypeEnum.JUXTAGLOMERULAR_GRANULAR_CELL); setActiveTab('4');Also applies to: 36-36, 41-41
62-62: ⚡ Quick winRemove debugging console.log statement.
This console.log statement appears to be a debugging artifact and should be removed before merging.
🧹 Proposed fix
const cellType = svgToCellMap[svgId]; - console.log(`Hovered over SVG group: ${svgId}, mapped to cell type: ${cellType}`); if (cellType) {src/components/Explorer/TabSection.js (2)
22-23: ⚡ Quick winInconsistent state management between top-level and nested buttons.
The top-level button's
onMouseEnterhandler (line 72) now only callshandleSchematicHoverEnterwithout updating local state, while the cell type (line 22) and subregion (line 35) buttons still update both parent state and localactiveCellstate viasetState. This inconsistency may be intentional, but if the goal is to centralize state management in the parent component, all buttons should follow the same pattern.Also applies to: 35-36, 72-72
96-96: 💤 Low valueVerify citation access date.
The citation indicates "Accessed on June 1, 2026" but the current date is June 2, 2026. Consider updating to the current date or verify if June 1 is the intended access date.
src/components/Explorer/AccordionTabSection.js (1)
132-132: 💤 Low valueVerify citation access date.
The citation indicates "Accessed on June 1, 2026" but the current date is June 2, 2026. Consider updating to the current date or verify if June 1 is the intended access date.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8ae8fd5-5a66-4e59-b7a2-49519eebbe80
📒 Files selected for processing (7)
src/components/Explorer/AccordionTabSection.jssrc/components/Explorer/CellTypeEnum.jssrc/components/Explorer/CellTypeTabs.jssrc/components/Explorer/GlomerulusSchematic.jssrc/components/Explorer/TabSection.jssrc/components/Explorer/TubuleSchematic.jssrc/helpers/Utils.js
| default: | ||
| let ontologyId = event.detail.representation_of; | ||
| ontologyId = ontologyId.replace('http://purl.obolibrary.org/obo/', '').replace(/_/g, ':'); | ||
| // Find the matching object in hubmapTermMap | ||
| const hubmapTermMap = await fetchHubmapTermMap(); | ||
| hubmapTermMap.forEach(obj => { | ||
| if (obj.hubmapOntologyId === ontologyId) { | ||
| handleCellTypeClick(obj.cellType); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Wrap default case in a block to restrict variable scope.
The variable declarations ontologyId and hubmapTermMap in the default case should be wrapped in curly braces to prevent potential access from other switch cases due to JavaScript's block scoping rules.
🔧 Proposed fix to add block scope
default:
+ {
let ontologyId = event.detail.representation_of;
ontologyId = ontologyId.replace('http://purl.obolibrary.org/obo/', '').replace(/_/g, ':');
// Find the matching object in hubmapTermMap
const hubmapTermMap = await fetchHubmapTermMap();
hubmapTermMap.forEach(obj => {
if (obj.hubmapOntologyId === ontologyId) {
handleCellTypeClick(obj.cellType);
}
});
+ break;
+ }
- }🧰 Tools
🪛 Biome (2.4.16)
[error] 46-46: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
[error] 49-49: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
3 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation