Rework package display view in Explore tab#480
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR migrates raster ChangesIcon Migration, Badge Component, and Explore List View
Sequence Diagram(s)sequenceDiagram
participant User
participant ExploreView
participant ExploreController
participant PackageListRowView
participant PackageBlocView
rect rgba(100, 149, 237, 0.5)
note over User,ExploreView: Tab switch / search
User->>ExploreView: Click list or grid tab
ExploreView->>ExploreController: tab selection changed
ExploreController->>ExploreController: toggle listPane / masonryPane visibility
end
User->>ExploreView: Apply filter / search
ExploreView->>ExploreController: performPackageSearch()
ExploreController->>ExploreController: displayPackages(): clear both views, reset partition counters
ExploreController->>PackageBlocView: displayNewGridPartition() — append grid cards
ExploreController->>PackageListRowView: displayNewListPartition() — append list rows
rect rgba(255, 165, 0, 0.5)
note over User,ExploreController: Lazy loading
User->>ExploreView: Scroll to bottom or click "Load more"
ExploreView->>ExploreController: vvalueProperty listener / hyperlink click
ExploreController->>PackageListRowView: displayNewListPartition() next partition
ExploreController->>PackageBlocView: displayNewGridPartition() next partition
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
owlplug-client/src/main/java/com/owlplug/explore/controllers/PackageInfoController.java (1)
195-200:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
typeLabelfor unknown/null plugin types.When type is not
INSTRUMENTorEFFECT, the label text is left unchanged, so stale text from a previous package can be shown.Suggested fix
if (remotePackage.getType() == PluginType.INSTRUMENT) { this.typeLabel.setText("Instrument (VSTi)"); } else if (remotePackage.getType() == PluginType.EFFECT) { this.typeLabel.setText("Effect (VST)"); + } else { + this.typeLabel.setText("Unknown type"); } this.typeLabel.setGraphic(this.getApplicationDefaults().getPackageTypeIcon(remotePackage.getType()));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@owlplug-client/src/main/java/com/owlplug/explore/controllers/PackageInfoController.java` around lines 195 - 200, The typeLabel text is only set when the plugin type matches INSTRUMENT or EFFECT, but is left unchanged for other or null types, causing stale text from previously displayed packages to remain visible. Add an else clause after the if-else chain that checks for INSTRUMENT and EFFECT types in the type-checking block to reset the typeLabel text to an appropriate default value (such as an empty string or a generic label). Ensure the icon is still set using getApplicationDefaults().getPackageTypeIcon() with the remote package type regardless of which branch executes.owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java (1)
329-344:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLazy-load bars are not reset when no partition is available.
lazyLoadBar/listLazyLoadBarvisibility is only updated inside the “append partition” branch. For empty result sets, stale “Load more...” visibility can persist from prior searches.Also applies to: 355-374
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java` around lines 329 - 344, The lazyLoadBar visibility is only updated when new partitions are available to display, which means for empty result sets the lazy-load bar can retain stale visibility from previous searches. Move the lazyLoadBar visibility logic outside the conditional block that checks if loadedPackagePartitions has more items than displayedPartitions, so the visibility is always updated to reflect the current state: hidden when no partitions are available and visible only when more partitions exist to load. Apply this same fix to the similar lazy-load bar visibility handling mentioned in the consolidated comment sites, ensuring both the primary displayNewPackagePartition method and any other affected partition display methods properly reset lazy-load bar visibility for empty results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@owlplug-client/src/main/java/com/owlplug/core/components/ApplicationDefaults.java`:
- Around line 160-165: The switch statement in getPackageTypeIcon(PluginType
type) method throws a NullPointerException when the type parameter is null. The
call in PackageInfoController.java at line 200 invokes this method with
remotePackage.getType() without null checking, unlike the null-guarded calls in
PackageListRowView and PackageBlocView. Add a null case to handle this scenario
within the switch statement in getPackageTypeIcon, or alternatively add a null
check at the PackageInfoController call site before invoking getPackageTypeIcon
to match the pattern already used in the other caller classes.
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/dialogs/InstallStepDialogController.java`:
- Around line 394-405: The reset() method in InstallStepDialogController clears
several text fields but does not clear the installationDirectoryTextField,
leaving stale installation path text visible when the dialog is reopened. Add a
call to installationDirectoryTextField.setText("") in the reset() method to
clear the installation directory path along with the other field resets like
installationDirectoryText, directoryValidText, directoryOverrideText, and
formatText.
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java`:
- Around line 351-352: The resultCounter text update currently only occurs in
the displayNewPackagePartition() method (around lines 351-352), so when the list
view loads additional partitions through pagination (in the code section at
lines 354-375), the counter is not updated and becomes stale. Extract the
resultCounter update logic into a separate method that can be called from both
partition loading paths, then call this new method from both
displayNewPackagePartition() and the list view pagination handling code to
ensure the counter stays synchronized regardless of which view loads new
partitions.
In
`@owlplug-client/src/main/java/com/owlplug/explore/model/search/ExploreFilterCriteria.java`:
- Line 29: Change the icon field in ExploreFilterCriteria from storing a
FontIcon node instance to storing the icon as a String literal. Update the
getIcon() method (and any related getter) to create and return a fresh FontIcon
instance from the stored string on each call, ensuring each render of the filter
in suggestions and selected chips gets its own independent node instance. Update
any setter methods and initialization code to accept and store the string
identifier instead of the node object.
In `@owlplug-client/src/main/java/com/owlplug/explore/ui/PackageBundlesView.java`:
- Around line 73-76: The for loop iterating over bundle.getFormats() lacks a
null check, which can cause a NullPointerException at runtime if the method
returns null. Add a null-check guard before the for loop to ensure
bundle.getFormats() is not null before attempting to iterate over it.
In `@owlplug-client/src/main/java/com/owlplug/explore/ui/PackageListRowView.java`:
- Around line 196-197: The version label in PackageListRowView is being created
by concatenating "v" with remotePackage.getVersion() without checking for null,
which produces the broken string "vnull" when version is missing. Add a null
check before creating the versionLabel to handle the case where
remotePackage.getVersion() returns null, either by setting an appropriate
default label text or conditionally creating the label only when a valid version
exists.
- Around line 225-227: The code creates a Menu called "Other Packages" and
immediately iterates over remotePackage.getBundles() in a for loop without
checking if the method returns null. When metadata is incomplete, getBundles()
can return null, causing a NullPointerException. Add a null-check guard before
the for loop to verify that remotePackage.getBundles() is not null before
attempting to iterate over its contents. If the bundles are null, either skip
processing the "Other Packages" menu entirely or handle it with a fallback
approach appropriate to the UI logic.
In
`@owlplug-client/src/main/java/com/owlplug/plugin/ui/PluginFormatBadgeView.java`:
- Around line 35-40: The string-based constructors of PluginFormatBadgeView are
passing formatValue directly to PluginFormat.fromBundleString(...) without null
or blank checks, which can cause exceptions at runtime during badge rendering.
Add validation in both constructors (the one at lines 35-40 taking formatValue
and applicationDefaults, and the one at lines 71-75 taking formatValue,
applicationDefaults, and DisplayMode mode) to check if formatValue is null or
empty before passing it to fromBundleString(). For null or blank values, provide
a safe default format value or handle it appropriately to prevent exceptions
from breaking badge creation in list/tree/package views.
In `@owlplug-client/src/main/resources/fxml/explore/PackageInfoView.fxml`:
- Around line 103-105: The FontIcon element under the typeLabel Label is not
wrapped in a <graphic> block, which prevents it from being recognized as the
graphic property per JavaFX FXML specification. Locate the typeLabel Label
element and the FontIcon child with iconLiteral="mdi2w-waveform", then wrap the
FontIcon element in opening and closing <graphic> tags to ensure the icon
displays correctly. Reference the other properly formatted Label and Button icon
assignments elsewhere in the file for the correct pattern.
In `@owlplug-client/src/main/resources/owlplug.css`:
- Line 165: CSS type selectors must use lowercase letters to satisfy stylelint
requirements. In the owlplug.css file, replace all uppercase `Label` type
selectors with lowercase `label` in the CSS rules that contain the `.chip-view
.chip` selector pattern. This includes updating the Label selectors to label
wherever they appear in similar selector chains throughout the file to ensure
consistency and compliance with the linting rules.
---
Outside diff comments:
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java`:
- Around line 329-344: The lazyLoadBar visibility is only updated when new
partitions are available to display, which means for empty result sets the
lazy-load bar can retain stale visibility from previous searches. Move the
lazyLoadBar visibility logic outside the conditional block that checks if
loadedPackagePartitions has more items than displayedPartitions, so the
visibility is always updated to reflect the current state: hidden when no
partitions are available and visible only when more partitions exist to load.
Apply this same fix to the similar lazy-load bar visibility handling mentioned
in the consolidated comment sites, ensuring both the primary
displayNewPackagePartition method and any other affected partition display
methods properly reset lazy-load bar visibility for empty results.
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/PackageInfoController.java`:
- Around line 195-200: The typeLabel text is only set when the plugin type
matches INSTRUMENT or EFFECT, but is left unchanged for other or null types,
causing stale text from previously displayed packages to remain visible. Add an
else clause after the if-else chain that checks for INSTRUMENT and EFFECT types
in the type-checking block to reset the typeLabel text to an appropriate default
value (such as an empty string or a generic label). Ensure the icon is still set
using getApplicationDefaults().getPackageTypeIcon() with the remote package type
regardless of which branch executes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1348eaaa-e565-4072-9f2f-9b7ce927e503
⛔ Files ignored due to path filters (25)
owlplug-client/src/main/resources/icons/arrow-down-grey-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/arrow-down-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/arrow-ui-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/box-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/box-yellow-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/circleplus-grey-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/download-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/earth-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/effect-white-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/filesystem-grey-48.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/gear-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/package-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/picture-grey-240.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/plus-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/refresh-white-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/rocket-white-64.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/search-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/server-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/share-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/soundwave-blue-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/synth-white-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/tag-white-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/user-white-32.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/verified-black-16.pngis excluded by!**/*.png,!**/*.pngowlplug-client/src/main/resources/icons/wave-circle-32.pngis excluded by!**/*.png,!**/*.png
📒 Files selected for processing (28)
owlplug-client/src/main/java/com/owlplug/core/components/ApplicationDefaults.javaowlplug-client/src/main/java/com/owlplug/core/controllers/dialogs/WelcomeDialogController.javaowlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.javaowlplug-client/src/main/java/com/owlplug/explore/controllers/NewSourceDialogController.javaowlplug-client/src/main/java/com/owlplug/explore/controllers/PackageInfoController.javaowlplug-client/src/main/java/com/owlplug/explore/controllers/dialogs/InstallStepDialogController.javaowlplug-client/src/main/java/com/owlplug/explore/model/search/ExploreFilterCriteria.javaowlplug-client/src/main/java/com/owlplug/explore/ui/ExploreChipView.javaowlplug-client/src/main/java/com/owlplug/explore/ui/PackageBlocView.javaowlplug-client/src/main/java/com/owlplug/explore/ui/PackageBundlesView.javaowlplug-client/src/main/java/com/owlplug/explore/ui/PackageListRowView.javaowlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginInfoController.javaowlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTableController.javaowlplug-client/src/main/java/com/owlplug/plugin/ui/PluginFormatBadgeView.javaowlplug-client/src/main/java/com/owlplug/plugin/ui/PluginListCellFactory.javaowlplug-client/src/main/java/com/owlplug/plugin/ui/PluginTreeCell.javaowlplug-client/src/main/java/com/owlplug/plugin/ui/RecoveredPluginView.javaowlplug-client/src/main/java/com/owlplug/project/controllers/ProjectInfoController.javaowlplug-client/src/main/resources/fxml/MainView.fxmlowlplug-client/src/main/resources/fxml/OptionsView.fxmlowlplug-client/src/main/resources/fxml/dialogs/InstallStepView.fxmlowlplug-client/src/main/resources/fxml/dialogs/NewSourceView.fxmlowlplug-client/src/main/resources/fxml/explore/ExploreView.fxmlowlplug-client/src/main/resources/fxml/explore/PackageInfoView.fxmlowlplug-client/src/main/resources/fxml/menu/SourceMenu.fxmlowlplug-client/src/main/resources/fxml/plugins/PluginInfoView.fxmlowlplug-client/src/main/resources/owlplug.cssowlplug-controls/src/main/resources/css/owlplug-controls.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.