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 list panels and first UI prototype #66

Merged
merged 9 commits into from Sep 28, 2019

Conversation

EugeneTeu
Copy link

Created PhoneListPanel and PhoneCard
Adjusted UI

@@ -13,14 +13,14 @@
SIZE_512GB("512GB"),
SIZE_1024GB("1024GB");

private final String label;
public final String value;
Copy link
Author

Choose a reason for hiding this comment

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

@uberSaiyan as discussed, switched this to value to keep with convention in PhoneCard

@@ -1,4 +1,4 @@
package seedu.address.ui;
package seedu.address.ui.cards;
Copy link
Author

Choose a reason for hiding this comment

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

Moved to Package Cards

@@ -0,0 +1,83 @@
package seedu.address.ui.cards;
Copy link
Author

Choose a reason for hiding this comment

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

same as above

@@ -0,0 +1,48 @@
package seedu.address.ui.panels;
Copy link
Author

Choose a reason for hiding this comment

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

moved to package Panels

Copy link

@qiujiaaa qiujiaaa left a comment

Choose a reason for hiding this comment

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

overall good job

Copy link

@zhixianggg zhixianggg left a comment

Choose a reason for hiding this comment

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

I think we should change the FXML files for PhoneCard as well

@@ -32,7 +32,7 @@
@FXML
private Label name;
@FXML
private Label id;
private Label index;
Copy link
Author

Choose a reason for hiding this comment

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

@qiujiaaa as discussed

Copy link

@yan-wl yan-wl left a comment

Choose a reason for hiding this comment

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

Good job

* Panel containing the list of phones.
*/
public class PhoneListPanel extends UiPart<Region> {
private static final String FXML = "PersonListPanel.fxml";
Copy link

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 updated yet?


/**
* Panel containing the list of persons.
* Panel containing the list of customers.
*/
public class CustomerListPanel extends UiPart<Region> {
private static final String FXML = "PersonListPanel.fxml";
Copy link

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 updated yet?

@EugeneTeu EugeneTeu removed the request for review from yeodonghan September 28, 2019 15:32
Copy link

@zhixianggg zhixianggg left a comment

Choose a reason for hiding this comment

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

i think mostly good

</HBox>
<FlowPane fx:id="tags" />
<Label fx:id="contactNumber" styleClass="cell_small_label" text="\$contactNumber" />
<Label fx:id="email" styleClass="cell_small_label" text="\$email" />

Choose a reason for hiding this comment

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

I think need to update right? haha all the labels

@EugeneTeu EugeneTeu merged commit b9a35e6 into AY1920S1-CS2103T-T09-4:master Sep 28, 2019
@EugeneTeu EugeneTeu self-assigned this Oct 4, 2019
@EugeneTeu EugeneTeu added this to the v1.2 milestone Oct 9, 2019
@EugeneTeu EugeneTeu added the priority.High Must do label Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants