-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update Help Window #180
Update Help Window #180
Conversation
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.
LGTM! Only minor fixes on UI/UX and making info match our UG! :D
@@ -117,23 +88,26 @@ private void initializeCommandTable() { | |||
"Automatic", | |||
"No command required"), |
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.
Should we swap the order of commands by its importance, reflecting what we order in the UG
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.
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.
Yup, I think we should follow this (or even consider sorting the commands based on whether they modify data or are purely for viewing).
Saving data should be moved to the bottom as well since users are unlikely to reference it often.
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.
👍
new CommandSummary("Find a Customer by Details", | ||
"edit 69 n/ TAN LESHEW p/ 77337733 e/ [email protected] a/ COM3 j/ doctor i/ 1000000000 " | ||
+ "t/ bronze s/ none ra/ Specialist in eye care"), | ||
new CommandSummary("Find a Client by Details", |
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.
Further from the UG, it uses "Filter Client List" for action column. Do you think that we should standardize?
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 we should. I'll follow the UG
+ "margin: 0; padding: 0; background-color: #2b2b2b;'>" | ||
+ "<p style='color: #e6e6e6; font-family: \"Segoe UI\", sans-serif; font-size: 14pt; " | ||
+ "text-align: center;'>" | ||
+ "For more information, please refer to the user guide." |
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.
@@ -61,9 +61,6 @@ dependencies { | |||
implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'win' | |||
implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'mac' | |||
implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'linux' | |||
implementation group: 'org.openjfx', name: 'javafx-web', version: javaFxVersion, classifier: 'win' | |||
implementation group: 'org.openjfx', name: 'javafx-web', version: javaFxVersion, classifier: 'mac' | |||
implementation group: 'org.openjfx', name: 'javafx-web', version: javaFxVersion, classifier: 'linux' |
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.
RIP mr.webhosting
Updates the help window to show the new commands/ changes to old commands and removes JavaFX WebView dependencies.
Closes #174