Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the track border drawing mechanism in IGV, moving from individual track-level border rendering to a centralized approach managed by the DataPanelPainter. The changes consolidate border preferences and implement a new configurable border drawing system.
Changes:
- Introduced new centralized preference settings for track borders (
TRACK.DRAW_BORDERSandTRACK.BORDER_COLOR) - Removed individual border drawing logic from multiple track renderers (FeatureTrack, CoverageTrack, XYPlotRenderer, IGVFeatureRenderer)
- Implemented centralized border rendering in DataPanelPainter with improved graphics clipping
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/igv/ui/panel/DataPanelPainter.java | Implements centralized border drawing logic with configurable preferences and improved graphics clipping |
| src/main/java/org/igv/ui/panel/DataPanel.java | Removes old border drawing code and commented out component border |
| src/main/java/org/igv/prefs/Constants.java | Adds new track border constants and reorganizes border-related preferences |
| src/main/java/org/igv/prefs/IGVPreferences.java | Adds preference migration logic from old border preferences and removes default constructor |
| src/main/resources/preferences.tab | Updates preference definitions to reflect new border settings |
| src/main/java/org/igv/track/FeatureTrack.java | Removes track-specific border drawing methods and static drawBorder field |
| src/main/java/org/igv/track/DataTrack.java | Adds dark mode support for zoom-in text color |
| src/main/java/org/igv/track/SequenceTrack.java | Adds gap spacing for translated sequences |
| src/main/java/org/igv/sam/CoverageTrack.java | Removes border drawing and adds dark mode text color support |
| src/main/java/org/igv/sam/AlignmentRenderer.java | Adds dark mode support for group divider color |
| src/main/java/org/igv/renderer/XYPlotRenderer.java | Removes border drawing logic and fixes yLineGraphics reference |
| src/main/java/org/igv/renderer/SequenceRenderer.java | Adds gap constant and applies it to translated sequence positioning |
| src/main/java/org/igv/renderer/IGVFeatureRenderer.java | Removes drawBoundary field and related rendering code |
| src/main/java/org/igv/renderer/GenotypeRenderer.java | Removes drawBoundary initialization |
| src/main/java/org/igv/renderer/DynSeqRenderer.java | Removes unused import |
| src/test/java/org/igv/sam/AlignmentTileLoaderTest.java | Updates test to use new IGVPreferences constructor |
| test/sessions/schatz_skbr3_pacbio_sampled.xml | Updates test session file with new viewport and track configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.parent = parent; | ||
| this.defaults = defaults; | ||
| this.userPreferences = userPreferences == null ? new HashMap<>() : userPreferences; | ||
| migrateUserPreferences(); |
There was a problem hiding this comment.
The migration is called in the constructor, which means it runs every time an IGVPreferences instance is created. Consider whether this should only run once globally or if migration state should be tracked to avoid repeated execution.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| private void init() { | ||
| setBorder(javax.swing.BorderFactory.createLineBorder(new java.awt.Color(200, 200, 200))); | ||
| // setBorder(javax.swing.BorderFactory.createLineBorder(new java.awt.Color(200, 200, 200))); |
There was a problem hiding this comment.
Commented-out code should be removed rather than left in the codebase. If this border functionality might be needed in the future, it should be documented in a commit message or issue tracker, not left as commented code.
| // setBorder(javax.swing.BorderFactory.createLineBorder(new java.awt.Color(200, 200, 200))); |
| public static final Color GROUP_GAP_COLOR = Globals.isDarkMode() ? Color.darkGray : UIConstants.LIGHT_GREY; | ||
| private static Logger log = LogManager.getLogger(AttributePanel.class); | ||
|
|
||
|
|
There was a problem hiding this comment.
The constant GROUP_GAP_COLOR is defined but never used in the visible diff. If this constant is intended to replace uses of UIConstants.LIGHT_GREY in this file, ensure all references are updated. The code at line 112 still uses GROUP_BORDER_COLOR instead.
| public static final Color GROUP_GAP_COLOR = Globals.isDarkMode() ? Color.darkGray : UIConstants.LIGHT_GREY; | |
| private static Logger log = LogManager.getLogger(AttributePanel.class); | |
| private static Logger log = LogManager.getLogger(AttributePanel.class); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
No description provided.