-
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
Fix code quality for logic package #362
Fix code quality for logic package #362
Conversation
Codecov Report
@@ Coverage Diff @@
## master #362 +/- ##
============================================
+ Coverage 89.54% 89.55% +0.01%
+ Complexity 996 995 -1
============================================
Files 112 112
Lines 2583 2586 +3
Branches 304 304
============================================
+ Hits 2313 2316 +3
Misses 183 183
Partials 87 87
Continue to review full report at Codecov.
|
widgetPlaceholder.getChildren().clear(); | ||
widgetPlaceholder.getChildren().add(widgetViewBox.getRoot()); |
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.
Bugfix for when country note view
is called followed by client list
, which doesn't reset the widget display viewbox back to default, even though it should.
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.
The suggested stuff should probably be reconsidered in the interest of clarity, but other than that almost LGTM.
src/main/java/seedu/address/logic/commands/ClientEditCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/seedu/address/logic/parser/ClientFindCommandParser.java
Outdated
Show resolved
Hide resolved
widgetPlaceholder.getChildren().clear(); | ||
widgetPlaceholder.getChildren().add(widgetViewBox.getRoot()); |
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.
LGTM
Description
As per title. Also fixed a small bug where
client list
orclient suggest
doesn't change the widget view box back to the default view when in country note view.Fixes #325
Testing
NIL
Remarks
NIL