-
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 UI and add view feature #108
Update UI and add view feature #108
Conversation
Codecov Report
@@ Coverage Diff @@
## master #108 +/- ##
============================================
- Coverage 77.84% 76.33% -1.52%
- Complexity 599 612 +13
============================================
Files 87 91 +4
Lines 1729 1796 +67
Branches 229 235 +6
============================================
+ Hits 1346 1371 +25
- Misses 327 369 +42
Partials 56 56
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The codecov failes is because most of the misses came from GUI and it is very hard to test. |
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.
Overall, LGTM 👍
<Label fx:id="name" styleClass="cell_big_label" text="\$first" wrapText="true" /> | ||
</children> | ||
</HBox> | ||
<FlowPane fx:id="languages" /> |
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.
just a slight suggestion for consistency maybe can put 'Languages: ' and 'Tags' in front
What's change:
Functional
Logic.java, LogicManager.java
ViewCommand.java, ViewCommandParser.java
AddressBookParser.java
Model.java, ModalManager.java
Add test for ViewCommand and ViewCommandParser
UI/UX
Update MainWindow:
Add personDetailCard, personDetailPane: to display the detailed information of the person that is selected
Minor change on the CSS