-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Added Settings menu to Drifty GUI #544
feat: Added Settings menu to Drifty GUI #544
Conversation
…plash dark png and make the dark theme apply on them
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes integrate a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Drifty_GUI
participant Settings
User->>+Drifty_GUI: Open Settings Menu
Drifty_GUI->>+Settings: Show Settings Window
Settings->>+User: Display Preferences
User->>+Settings: Set Preferences (e.g., Theme, Auto-Paste)
Settings->>+User: Save Preferences
User->>+Drifty_GUI: See Updated Preferences
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! You did it 🎉 Now, Relax 😉, Grab a drink ☕, and wait for the maintainers to check your contributions. Meanwhile, you can discuss on other issues and solve them 😀. Thank You 😃!
Meanwhile you can also discuss about the project in our Discord Server 😀
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (6)
GUI/src/main/resources/Backgrounds/DriftyMain Dark.png
is excluded by!**/*.png
GUI/src/main/resources/Buttons/Save/SaveDown Dark.png
is excluded by!**/*.png
GUI/src/main/resources/Buttons/Save/SaveUp Dark.png
is excluded by!**/*.png
GUI/src/main/resources/Buttons/Start/StartDown Dark.png
is excluded by!**/*.png
GUI/src/main/resources/Buttons/Start/StartUp Dark.png
is excluded by!**/*.png
GUI/src/main/resources/Splash Dark.png
is excluded by!**/*.png
Files selected for processing (14)
- .idea/inspectionProfiles/Project_Default.xml (1 hunks)
- Core/src/main/java/preferences/Get.java (2 hunks)
- GUI/src/main/java/gui/preferences/Clear.java (1 hunks)
- GUI/src/main/java/gui/preferences/Get.java (2 hunks)
- GUI/src/main/java/gui/preferences/Labels.java (1 hunks)
- GUI/src/main/java/gui/preferences/Set.java (2 hunks)
- GUI/src/main/java/gui/support/Constants.java (4 hunks)
- GUI/src/main/java/main/Drifty_GUI.java (6 hunks)
- GUI/src/main/java/ui/ConfirmationDialog.java (1 hunks)
- GUI/src/main/java/ui/MainGridPane.java (4 hunks)
- GUI/src/main/java/ui/Settings.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (5 hunks)
- GUI/src/main/resources/CSS/DarkTheme.css (1 hunks)
- GUI/src/main/resources/CSS/LightTheme.css (1 hunks)
Files skipped from review due to trivial changes (3)
- .idea/inspectionProfiles/Project_Default.xml
- GUI/src/main/resources/CSS/DarkTheme.css
- GUI/src/main/resources/CSS/LightTheme.css
Additional context used
GitHub Check: CodeQL
GUI/src/main/java/main/Drifty_GUI.java
[notice] 219-219: Unread local variable
Variable 'Settings settings1' is never read.
[notice] 78-78: Useless toString on String
Redundant call to 'toString' on a String object.
[notice] 189-189: Useless toString on String
Redundant call to 'toString' on a String object.
[notice] 197-197: Useless toString on String
Redundant call to 'toString' on a String object.GUI/src/main/java/ui/Settings.java
[notice] 105-105: Useless toString on String
Redundant call to 'toString' on a String object.
Additional comments not posted (12)
GUI/src/main/java/gui/preferences/Labels.java (1)
4-4
: The addition ofMAIT_THEME
to theLabels
enum is correctly implemented and aligns with the PR objectives to manage theme settings.GUI/src/main/java/gui/preferences/Clear.java (1)
22-24
: The implementation ofmainTheme()
in theClear
class is correctly done, using theMAIT_THEME
constant to remove the theme preference.GUI/src/main/java/gui/preferences/Get.java (1)
47-49
: The implementation ofmainTheme()
correctly retrieves the theme preference using theMAIT_THEME
constant.Core/src/main/java/preferences/Get.java (2)
27-27
: Change to make the Preferences object static is appropriate for centralized management of preferences. Good refactoring!
37-37
: MakinglastDownloadFolder()
static enhances its usability across the application without needing an instance of the class. Well done!GUI/src/main/java/ui/ConfirmationDialog.java (1)
4-4
: Imports added are necessary for the new functionality around settings and theme management. This is well-aligned with the PR's objectives.Also applies to: 11-11, 18-18, 22-22
GUI/src/main/java/ui/MainGridPane.java (3)
31-31
: Conditional logic for applying themes to buttons enhances user experience by ensuring UI consistency. Great implementation!Also applies to: 34-34
155-155
: Centralizing the creation of HBoxes in a static method enhances consistency and maintainability. Good design choice!
167-167
: Centralizing the creation of ImageViews in a static method is a solid approach for maintainability and ease of adjustments.GUI/src/main/java/gui/support/Constants.java (1)
26-26
: Dynamic resource loading based on the theme settings is a critical enhancement for supporting theme switching. This change significantly improves flexibility and user experience.Also applies to: 30-30, 32-32, 35-35, 36-36, 42-42, 46-46, 47-47, 63-63, 64-64, 71-71, 72-72, 74-74, 76-76
GUI/src/main/java/ui/UIController.java (2)
100-105
: Conditional styling based on the theme setting.This segment correctly applies text styles based on the theme, enhancing the UI's responsiveness to theme changes. Good use of conditional logic to handle UI consistency across different themes.
625-625
: Implementation of theisAutoPaste
method.This method provides a concise and clear way to determine the auto-paste status by checking both the checkbox and a global setting. It's a good example of how to effectively use short-circuit logic for UI state checks.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- GUI/src/main/java/gui/preferences/Get.java (2 hunks)
- GUI/src/main/java/main/Drifty_GUI.java (6 hunks)
- GUI/src/main/java/ui/Settings.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/gui/preferences/Get.java
- GUI/src/main/java/ui/UIController.java
Additional context used
GitHub Check: CodeQL
GUI/src/main/java/main/Drifty_GUI.java
[notice] 218-218: Unread local variable
Variable 'Settings settings1' is never read.
[notice] 79-79: Useless toString on String
Redundant call to 'toString' on a String object.
[notice] 190-190: Useless toString on String
Redundant call to 'toString' on a String object.GUI/src/main/java/ui/Settings.java
[notice] 106-106: Useless toString on String
Redundant call to 'toString' on a String object.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- GUI/src/main/java/gui/support/Constants.java (4 hunks)
- GUI/src/main/java/ui/ConfirmationDialog.java (1 hunks)
- GUI/src/main/java/ui/Settings.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/gui/support/Constants.java
- GUI/src/main/java/ui/ConfirmationDialog.java
Additional context used
GitHub Check: CodeQL
GUI/src/main/java/ui/Settings.java
[notice] 102-102: Useless toString on String
Redundant call to 'toString' on a String object.
❌ Linting errors found!@ziad-ashraf7 Please fix the following errors: GUI/src/main/java/ui/UIController.java:94:9: Unused local variable 'disableStartButton'. [UnusedLocalVariable]
GUI/src/main/java/gui/support/Constants.java:26:23: Name 'DRIFTY_MAIN_PNG' must match pattern '^[a-z][a-zA-Z0-*$'. [StaticVariableName]
GUI/src/main/java/gui/support/Constants.java:63:25: Name 'IMG_MAIN_GUI_BANNER' must match pattern '^[a-z][a-zA-Z0-*$'. [StaticVariableName]
GUI/src/main/java/gui/support/Constants.java:64:25: Name 'IMG_SPLASH' must match pattern '^[a-z][a-zA-Z0-*$'. [StaticVariableName]
Core/src/main/java/preferences/Get.java:27:38: Name 'preferences' must match pattern '^[A-Z][A-Z0-*(_[A-Z0-9]+)*$'. [ConstantName] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- GUI/src/main/java/gui/preferences/Set.java (2 hunks)
- GUI/src/main/java/main/Drifty_GUI.java (6 hunks)
- GUI/src/main/java/ui/Settings.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (7 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/gui/preferences/Set.java
- GUI/src/main/java/ui/UIController.java
Additional context used
GitHub Check: CodeQL
GUI/src/main/java/main/Drifty_GUI.java
[notice] 79-79: Useless toString on String
Redundant call to 'toString' on a String object.
[notice] 190-190: Useless toString on String
Redundant call to 'toString' on a String object.GUI/src/main/java/ui/Settings.java
[notice] 102-102: Useless toString on String
Redundant call to 'toString' on a String object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- GUI/src/main/java/gui/utils/UIComponentBuilder.java (1 hunks)
- GUI/src/main/java/ui/Settings.java (1 hunks)
- GUI/src/main/resources/META-INF/native-image/jni-config.json (3 hunks)
- GUI/src/main/resources/META-INF/native-image/reflect-config.json (4 hunks)
- GUI/src/main/resources/META-INF/native-image/resource-config.json (6 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/gui/utils/UIComponentBuilder.java
- GUI/src/main/java/ui/Settings.java
Additional comments not posted (8)
GUI/src/main/resources/META-INF/native-image/jni-config.json (4)
95-98
: Validate the addition ofunmodifiableMap
injava.util.Collections
.The
unmodifiableMap
method is used to create an unmodifiable view of a given map, which is essential for immutability in multi-threaded environments. Ensure this method is necessary for the native parts of the application to maintain state consistency without modification.
75-78
: Check the necessity and usage ofjava.lang.Object.equals
in native code.The method
equals
forjava.lang.Object
is crucial for comparing objects in Java. If this is exposed to native code, verify that it's used appropriately to prevent incorrect comparisons or potential security issues.Verification successful
Check the necessity and usage of
java.lang.Object.equals
in native code.The
equals
method forjava.lang.Object
is used extensively throughout the codebase, but there is no evidence of it being exposed to or used in native code. The current changes appear to be safe and do not introduce any potential security issues related to native code usage.
- File:
CLI/src/main/java/main/Drifty_CLI.java
- File:
CLI/src/main/java/backend/FileDownloader.java
- File:
GUI/src/main/java/ui/Theme.java
- File:
GUI/src/main/java/ui/MainGridPane.java
- File:
GUI/src/main/java/ui/ConfirmationDialog.java
- File:
GUI/src/main/java/ui/GetFilename.java
- File:
GUI/src/main/java/ui/UIController.java
- File:
GUI/src/main/java/ui/GetConfirmationDialogResponse.java
- File:
GUI/src/main/java/ui/Settings.java
- File:
GUI/src/main/java/ui/About.java
- File:
GUI/src/main/java/gui/support/Constants.java
- File:
Core/src/main/java/utils/MessageBroker.java
- File:
Core/src/main/java/utils/Utility.java
- File:
Core/src/main/java/support/Job.java
- File:
Core/src/main/java/support/JobHistory.java
- File:
Core/src/main/java/properties/OS.java
- File:
Core/src/main/java/properties/Mode.java
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for native code usage of `java.lang.Object.equals`. # Test: Search for native usage of the equals method. Expect: Correct and secure usage. ast-grep --lang java --pattern $'JNI_method($_, $_) { $_.get("java.lang.Object.equals") }'Length of output: 570
Script:
#!/bin/bash # Description: Corrected script to check for native code usage of `java.lang.Object.equals`. # Test: Search for native usage of the equals method. Expect: Correct and secure usage. ast-grep --lang java --pattern $'MethodInvocation { expression: MemberReference { member: "equals" expression: TypeReference { type: "java.lang.Object" } } }'Length of output: 750
Script:
#!/bin/bash # Description: Simplified script to check for usages of `java.lang.Object.equals`. # Test: Search for any usage of the equals method in the codebase. Expect: Correct and secure usage. ast-grep --lang java --pattern $'MethodInvocation { expression: MemberReference { member: "equals" } }'Length of output: 612
Script:
#!/bin/bash # Description: Use ripgrep to check for usages of `equals` method in Java files. # Test: Search for any usage of the equals method in the codebase. Expect: Correct and secure usage. rg '\.equals\(' --glob '*.java'Length of output: 8536
67-70
: Ensure correct JNI configuration forjava.lang.Boolean
.The addition of
java.lang.Boolean
fieldsTRUE
andFALSE
could be necessary for native interactions where Boolean values are frequently used. Ensure that native code using these fields is properly handling Java Boolean types.
113-113
: Ensure appropriate use ofjava.util.Map
methods in native code.The addition of
put
and other methods fromjava.util.Map
suggests their use in native code, possibly for configuration or state management. Verify that these methods are used correctly to prevent data integrity issues.GUI/src/main/resources/META-INF/native-image/resource-config.json (3)
4-22
: Validate the inclusion of new theme resources.The addition of patterns for dark and light theme resources (
DriftyMainDark.png
,SaveDownDark.png
, etc.) aligns with the PR's objective to enhance theme management. Ensure that these resources are correctly referenced and accessible within the application.
60-62
: Ensure correct inclusion of new splash images.The new
QSplashDark.png
andQSplashLight.png
are added to support the theme dynamics. Confirm that these images are properly loaded and displayed during the application startup.
30-34
: Check the inclusion of new CSS files for themes.The patterns for
DarkTheme.css
andLightTheme.css
are crucial for supporting the new theming capabilities. Verify that these CSS files are properly linked and applied across the application's GUI components.Verification successful
Check the inclusion of new CSS files for themes.
The patterns for
DarkTheme.css
andLightTheme.css
are crucial for supporting the new theming capabilities. The search results confirm that these CSS files are referenced inGUI/src/main/resources/META-INF/native-image/resource-config.json
and inGUI/src/main/java/gui/support/Constants.java
.
resource-config.json
: The patterns forDarkTheme.css
andLightTheme.css
are correctly included.Constants.java
: The CSS files are referenced viaConstants.class.getResource("/CSS/LightTheme.css")
andConstants.class.getResource("/CSS/DarkTheme.css")
.These references indicate that the CSS files are properly linked within the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that new CSS files are properly linked and applied. # Test: Search for application of new CSS files in GUI components. Expect: Correct linkage and application. rg --type css 'DarkTheme.css|LightTheme.css'Length of output: 46
Script:
#!/bin/bash # Description: Verify that new CSS files are properly linked and applied in HTML and JavaScript files. # Test: Search for references to DarkTheme.css and LightTheme.css in HTML and JavaScript files. # Search in HTML files rg --type html 'DarkTheme.css|LightTheme.css' # Search in JavaScript files rg --type js 'DarkTheme.css|LightTheme.css'Length of output: 242
Script:
#!/bin/bash # Description: Verify that new CSS files are properly linked and applied in the codebase. # Test: Search for references to DarkTheme.css and LightTheme.css in all file types with debug information. # Search in all file types with debug information rg --debug 'DarkTheme.css|LightTheme.css'Length of output: 7087
GUI/src/main/resources/META-INF/native-image/reflect-config.json (1)
139-141
: Check the necessity of the newPPSBlend_SRC_INPeer
constructor.The addition of a constructor for
PPSBlend_SRC_INPeer
suggests enhancements in blending operations for the application's graphical interface. Verify that this new constructor is essential and correctly used in the context of theme management and UI updates.
❌ Linting errors found!@ziad-ashraf7 Please fix the following errors: GUI/src/main/java/ui/UIController.java:60:34: Name 'infoTf' must match pattern '^[A-Z][A-Z0-*(_[A-Z0-9]+)*$'. [ConstantName] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range and nitpick comments (2)
GUI/src/main/java/ui/UIController.java (2)
Line range hint
812-835
: Review layout and styling in help method.
The method sets up a complex UI layout programmatically, which can be hard to maintain. Consider using FXML for defining UI layouts to improve maintainability.
Line range hint
844-850
: Ensure consistent styling in text generation method.
The methodtext
dynamically creates styled text elements. Ensure that styles are consistent and consider extracting style settings to a configuration class or file to ease maintenance and adjustments.private Text createStyledText(String content, boolean isBold, Color textColor, double fontSize) { Text text = new Text(content); text.setFont(new Font("monospace", fontSize)); text.setFill(textColor); if (isBold) { text.setStyle("-fx-font-weight: bold;"); } return text; }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- GUI/src/main/java/ui/Settings.java (1 hunks)
- GUI/src/main/java/ui/Theme.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (11 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/ui/Settings.java
- GUI/src/main/java/ui/Theme.java
Additional comments not posted (2)
GUI/src/main/java/ui/UIController.java (2)
766-769
: Optimize job list management.
The method for removing duplicate jobs and sorting the job list can be optimized to reduce complexity and improve performance, especially with large job lists.
778-804
: Ensure accessibility and readability in help documentation.
The help method uses dynamic text styling, which is good for readability. However, ensure that all text is accessible, possibly by providing alternative text descriptions or ensuring compatibility with screen readers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- GUI/src/main/java/ui/Theme.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (11 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/ui/Theme.java
- GUI/src/main/java/ui/UIController.java
❌ Linting errors found!@ziad-ashraf7 Please fix the following errors: GUI/src/main/java/ui/UIController.java:72:28: Name 'getINFO_TF' must match pattern '^[a-z][a-zA-Z0-*$'. [MethodName] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- GUI/src/main/java/ui/Settings.java (1 hunks)
- GUI/src/main/java/ui/Theme.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (11 hunks)
Files skipped from review as they are similar to previous changes (3)
- GUI/src/main/java/ui/Settings.java
- GUI/src/main/java/ui/Theme.java
- GUI/src/main/java/ui/UIController.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- GUI/src/main/java/ui/Theme.java (1 hunks)
- GUI/src/main/java/ui/UIController.java (11 hunks)
Files skipped from review as they are similar to previous changes (2)
- GUI/src/main/java/ui/Theme.java
- GUI/src/main/java/ui/UIController.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- GUI/src/main/java/main/Drifty_GUI.java (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- GUI/src/main/java/main/Drifty_GUI.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge 👍.
Thanks for contributing 🚀 🚀.
Fixes issue
Fixes #282
Changes proposed
Check List (Check all the applicable boxes)
Screenshots
2024-06-20.21-14-03.mp4
Note to reviewers
Summary by CodeRabbit
New Features
Refactor
Style