-
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
Show Total Cost in UI #157
Conversation
@@ -234,6 +234,7 @@ public void setTravelPlanObject(TravelPlanObject target, TravelPlanObject edited | |||
requireAllNonNull(target, editedTravelPlanObject); | |||
assert directory instanceof TravelPlan : "Directory must be set to a TravelPlan to call setTravelPlanObject."; | |||
TravelPlan tp = (TravelPlan) directory; | |||
Directory d = tp; |
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.
What does this line suppose to do?
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.
deleted
private final Logger logger = LogsCenter.getLogger(TravelPlanPanel.class); | ||
|
||
private Directory directory; | ||
|
||
private Logic logic; |
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.
Why do you need logic here tho? Doesn't seem to be used anywhere in the changes in this class?
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.
deleted
@@ -39,11 +46,16 @@ public TravelPlanPanel(Directory directory) { | |||
public void update() { | |||
if (directory instanceof TravelPlan) { | |||
TravelPlan travelPlan = (TravelPlan) directory; | |||
String cost = travelPlan.getTotalCost(); |
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.
Now no need do typecasting already. You can add this getTotalCost() method into Directory class.
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.
But still need to check isTravelPlan() to setText lah
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.
ok solved
@@ -85,6 +87,7 @@ public void setAllTabsVisible() { | |||
@Override | |||
protected void updateItem(Activity activity, boolean empty) { | |||
super.updateItem(activity, empty); | |||
travelPlanPanel.update(); |
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.
Is calling the update() here suitable?
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.
No choice...
@@ -159,6 +159,15 @@ private void handleDirectoryChange(Directory directory) { | |||
} | |||
} | |||
|
|||
private void handleCostChange(Directory directory) { |
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.
I think the travelPlanPanel.update() can be called here instead? So, no need to pass into TravelPlanObjectListPanel
No description provided.