Skip to content
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

Add planner UI #196

Merged
merged 23 commits into from Oct 30, 2019
Merged

Add planner UI #196

merged 23 commits into from Oct 30, 2019

Conversation

nishcafe
Copy link

No description provided.

@nishcafe nishcafe self-assigned this Oct 30, 2019
@nishcafe nishcafe added feature.Planner priority.High Must do type.Task Something that needs to be done, but not a story, bug, or an epic labels Oct 30, 2019
<Label fx:id="taskDes" text="\$first" styleClass="cell_big_label" />
</HBox>
<FlowPane fx:id="tags" />
<Label fx:id="priority" styleClass="cell_small_label" text="\priority" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove \ from here and from the other Labels, Java is interpreting this as escaping a character. PersonListCard uses a $ inside theirs which they need to escape.

@@ -47,7 +49,7 @@
public static final String MESSAGE_INVERSE_SUCCESS_DELETE = "Deleted task: %1$s";
public static final String MESSAGE_INVERSE_TASK_NOT_FOUND = "Task already deleted: %1$s";

public static final boolean HAS_INVERSE = true;
public static final boolean HAS_INVERSE = false;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be false?

@@ -32,7 +34,7 @@
public static final String MESSAGE_INVERSE_SUCCESS_ADD = "New task added task: %1$s";
public static final String MESSAGE_INVERSE_TASK_TO_ADD_ALREADY_EXIST = "Task already added: %1$s";

public static final boolean HAS_INVERSE = true;
public static final boolean HAS_INVERSE = false;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - I think it's not supposed to be false.

@@ -35,7 +37,7 @@
public static final String MESSAGE_INVERSE_SUCCESS_UNDONE = "%1$s marked as undone.";
public static final String MESSAGE_INVERSE_TASK_ALREADY_UNDONE = "Task has already been marked as undone";

public static final boolean HAS_INVERSE = true;
public static final boolean HAS_INVERSE = false;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

*/
Todo,
Event,
Deadline

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure enum values should be in upper-case.

@@ -22,7 +22,7 @@
void hasInverseExecution_success() {
Task t = new Todo("borrow book");
AddTaskCommand command = new AddTaskCommand(t);
assertTrue(command.hasInverseExecution());
assertFalse(command.hasInverseExecution());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change test for this to reflect if you changed HAS_INVERSE.

@@ -36,7 +36,7 @@ void getTargetIndex() throws ParseException {
@Test
void hasInverseExecution() throws ParseException {
DeleteTaskCommand command = new DeleteTaskCommand(parseIndex("2"));
assertTrue(command.hasInverseExecution());
assertFalse(command.hasInverseExecution());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Save as above.

@@ -32,7 +32,7 @@ void getCommandWord() {
@Test
void hasInverseExecution() {
DoneTaskCommand command = new DoneTaskCommand(parseIndex("2"));
assertTrue(command.hasInverseExecution());
assertFalse(command.hasInverseExecution());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Copy link

@ryanYtan ryanYtan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly just changes from testing, and a bit of style.

@marcfyk marcfyk merged commit 2621b0b into AY1920S1-CS2103T-T10-1:master Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature.Planner priority.High Must do type.Task Something that needs to be done, but not a story, bug, or an epic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants