Skip to content

Commit

Permalink
PersonCard: Add colors to tag labels
Browse files Browse the repository at this point in the history
The tags are all displayed with the same color.

It is hard to differentiate between tags visually across contacts.
For example, it is hard to visually group all contacts tagged with
"colleague". It is also hard to differentiate between those contacts
that are tagged with "colleague", and those that are not tagged with
"colleague".

Let's add different colors to different tag labels. Tags of the same
name will always receive the same color, while different tag names
should receive different colors if possible.

The hashcode of the tag string is used to determine the tag color.
This will assign a reasonably random color to each tag, and will
also ensure that the tag color remains the same when the user
launches the application again in the future (assuming the
application version remains the same). The idea of using hashcode is
influenced by a developer's suggestion[1].

[1] nus-cs2103-AY1718S1/forum#199
  • Loading branch information
yamgent committed Jan 28, 2018
1 parent a36b637 commit a0f850b
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 5 deletions.
24 changes: 23 additions & 1 deletion src/main/java/seedu/address/ui/PersonCard.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
public class PersonCard extends UiPart<Region> {

private static final String FXML = "PersonListCard.fxml";
private static final String[] TAG_COLOR_STYLES =
{ "teal", "red", "yellow", "blue", "orange", "brown", "green", "pink", "black", "grey" };

/**
* Note: Certain keywords such as "location" and "resources" are reserved keywords in JavaFX.
Expand Down Expand Up @@ -47,7 +49,27 @@ public PersonCard(Person person, int displayedIndex) {
phone.setText(person.getPhone().value);
address.setText(person.getAddress().value);
email.setText(person.getEmail().value);
person.getTags().forEach(tag -> tags.getChildren().add(new Label(tag.tagName)));
initTags(person);
}

/**
* Returns the color style for {@code tagName}'s label.
*/
private String getTagColorStyleFor(String tagName) {
// we use the hash code of the tag name to generate a random color, so that the color remain consistent
// between different runs of the program while still making it random enough between tags.
return TAG_COLOR_STYLES[Math.abs(tagName.hashCode()) % TAG_COLOR_STYLES.length];
}

/**
* Creates the tag labels for {@code person}.
*/
private void initTags(Person person) {
person.getTags().forEach(tag -> {
Label tagLabel = new Label(tag.tagName);
tagLabel.getStyleClass().add(getTagColorStyleFor(tag.tagName));
tags.getChildren().add(tagLabel);
});
}

@Override
Expand Down
52 changes: 50 additions & 2 deletions src/main/resources/view/DarkTheme.css
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,58 @@
}

#tags .label {
-fx-text-fill: white;
-fx-background-color: #3e7b91;
-fx-padding: 1 3 1 3;
-fx-border-radius: 2;
-fx-background-radius: 2;
-fx-font-size: 11;
}

#tags .teal {
-fx-text-fill: white;
-fx-background-color: #3e7b91;
}

#tags .red {
-fx-text-fill: black;
-fx-background-color: red;
}

#tags .yellow {
-fx-background-color: yellow;
-fx-text-fill: black;
}

#tags .blue {
-fx-text-fill: white;
-fx-background-color: blue;
}

#tags .orange {
-fx-text-fill: black;
-fx-background-color: orange;
}

#tags .brown {
-fx-text-fill: white;
-fx-background-color: brown;
}

#tags .green {
-fx-text-fill: black;
-fx-background-color: green;
}

#tags .pink {
-fx-text-fill: black;
-fx-background-color: pink;
}

#tags .black {
-fx-text-fill: white;
-fx-background-color: black;
}

#tags .grey {
-fx-text-fill: black;
-fx-background-color: grey;
}
9 changes: 9 additions & 0 deletions src/test/java/guitests/guihandles/PersonCardHandle.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,13 @@ public List<String> getTags() {
.map(Label::getText)
.collect(Collectors.toList());
}

public List<String> getTagStyleClasses(String tag) {
return tagLabels
.stream()
.filter(label -> label.getText().equals(tag))
.map(Label::getStyleClass)
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("No such tag."));
}
}
55 changes: 53 additions & 2 deletions src/test/java/seedu/address/ui/testutil/GuiTestAssert.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
package seedu.address.ui.testutil;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import guitests.guihandles.PersonCardHandle;
import guitests.guihandles.PersonListPanelHandle;
import guitests.guihandles.ResultDisplayHandle;
import seedu.address.model.person.Person;
import seedu.address.ui.PersonCard;

/**
* A set of assertion methods useful for writing GUI tests.
*/
public class GuiTestAssert {
private static final String LABEL_DEFAULT_STYLE = "label";

/**
* Asserts that {@code actualCard} displays the same values as {@code expectedCard}.
*/
Expand All @@ -24,6 +29,9 @@ public static void assertCardEquals(PersonCardHandle expectedCard, PersonCardHan
assertEquals(expectedCard.getName(), actualCard.getName());
assertEquals(expectedCard.getPhone(), actualCard.getPhone());
assertEquals(expectedCard.getTags(), actualCard.getTags());

expectedCard.getTags().forEach(tag ->
assertEquals(expectedCard.getTagStyleClasses(tag), actualCard.getTagStyleClasses(tag)));
}

/**
Expand All @@ -34,8 +42,51 @@ public static void assertCardDisplaysPerson(Person expectedPerson, PersonCardHan
assertEquals(expectedPerson.getPhone().value, actualCard.getPhone());
assertEquals(expectedPerson.getEmail().value, actualCard.getEmail());
assertEquals(expectedPerson.getAddress().value, actualCard.getAddress());
assertEquals(expectedPerson.getTags().stream().map(tag -> tag.tagName).collect(Collectors.toList()),
actualCard.getTags());

assertTagsEqual(expectedPerson, actualCard);
}

/**
* Returns the color style for {@code tagName}'s label. The tag's color is determined by looking up the color
* in {@code PersonCard#TAG_COLOR_STYLES}, using an index generated by the hash code of the tag's content.
*
* @see PersonCard#getTagColorStyleFor(String)
*/
private static String getTagColorStyleFor(String tagName) {
switch (tagName) {
case "classmates":
return "teal";
case "colleagues":
return "yellow";
case "family":
return "orange";
case "friends":
return "brown";
case "friend":
return "orange";
case "husband":
return "grey";
case "neighbours":
return "yellow";
case "owesMoney":
return "teal";
default:
fail(tagName + " does not have a color assigned.");
return "";
}
}

/**
* Asserts that the tags in {@code actualCard} matches all the tags in {@code expectedPerson} with the correct
* color.
*/
private static void assertTagsEqual(Person expectedPerson, PersonCardHandle actualCard) {
List<String> expectedTags = expectedPerson.getTags().stream()
.map(tag -> tag.tagName).collect(Collectors.toList());
assertEquals(expectedTags, actualCard.getTags());
expectedTags.forEach(tag ->
assertEquals(Arrays.asList(LABEL_DEFAULT_STYLE, getTagColorStyleFor(tag)),
actualCard.getTagStyleClasses(tag)));
}

/**
Expand Down

0 comments on commit a0f850b

Please sign in to comment.