Ui: Show total persons in status bar #803

yamgent · 2018-01-19T11:55:52Z

Part of #784. Rebased, and added tests.

Proposed merge commit message:

The status bar does not show how many people are in the address book.
This information could be useful for the user when the user wants to
judge the size of the address book.

Let's modify the status bar to include the total amount of people in the
address book.

Note: For demo purpose, don't merge this to master!

CanIHasReview-bot · 2018-01-19T12:01:08Z

v1

@yamgent submitted v1 for review.

[v1 1/2] StatusBarFooter: Show total persons
[v1 2/2] AddressBookSystemTest: Verify that total person status is updated

(📚 Archive)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/803/1/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

Zhiyuan-Amos

[1/2] 💯

Commit message can be updated though. For paragraph 3: "Let's modify StatusBarFooter so that it shows the total number of persons in the current address book, so that the user knows the id of the last person.

Zhiyuan-Amos

[2/2] why no commit message? :P

Zhiyuan-Amos · 2018-01-20T02:35:22Z

src/test/java/systemtests/AddressBookSystemTest.java

+        String expectedSyncStatus = String.format(SYNC_STATUS_UPDATED, timestamp);
+        assertEquals(expectedSyncStatus, handle.getSyncStatus());
+
+        assertEquals(String.format(TOTAL_PERSONS_STATUS, totalPersons), handle.getTotalPersonsStatus());


we can replace totalPersons with testApp.getModel().getFilteredPersonList().size()

yamgent · 2018-01-20T05:29:41Z

Note to self: AddressBookChangedEvent does not have filtered list.

CanIHasReview-bot · 2018-02-09T04:04:09Z

v2

@yamgent submitted v2 for review.

[v2 1/2] StatusBarFooter: Show total persons
[v2 2/2] AddressBookSystemTest: Verify that total person status is updated

(📚 Archive) (📈 Interdiff between v1 and v2)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/803/2/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

Zhiyuan-Amos

[2/2]

Zhiyuan-Amos · 2018-02-09T08:10:26Z

src/test/java/systemtests/AddressBookSystemTest.java

+     * {@code ClockRule#getInjectedClock()}, total persons was changed to match the total
+     * number of persons in the address book, while the save location remains the same.
+     */
+    protected void assertStatusBarUnchangedExceptSyncStatusAndTotalPersons() {


assertStatusBarChangedExceptSaveLocation? XD

Zhiyuan-Amos · 2018-02-09T08:11:26Z

src/test/java/systemtests/AddressBookSystemTest.java

+    }
+
+    /**
+     * Asserts that only the sync status in the status bar was changed to the timing of


the word only seems weird because 2 components in StatusBarFooter are updated.

Zhiyuan-Amos · 2018-02-09T08:14:48Z

src/main/java/seedu/address/ui/MainWindow.java

@@ -125,7 +125,8 @@ void fillInnerParts() {
        ResultDisplay resultDisplay = new ResultDisplay();
        resultDisplayPlaceholder.getChildren().add(resultDisplay.getRoot());

-        StatusBarFooter statusBarFooter = new StatusBarFooter(prefs.getAddressBookFilePath());
+        StatusBarFooter statusBarFooter = new StatusBarFooter(prefs.getAddressBookFilePath(),
+                logic.getFilteredPersonList().size());


this line looks weird because statusBarFooter actually stores the number of persons in the address book, not in the filtered person list :P But well, seems like we can't do anything about this.

CanIHasReview-bot · 2018-02-11T03:35:18Z

v3

@yamgent submitted v3 for review.

[v3 1/2] StatusBarFooter: Show total persons
[v3 2/2] AddressBookSystemTest: Verify that total person status is updated

(📚 Archive) (📈 Interdiff between v2 and v3)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/803/3/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

CanIHasReview-bot · 2018-08-09T05:59:00Z

v4

@yamgent submitted v4 for review.

[v4 1/2] StatusBarFooter: Show total persons
[v4 2/2] AddressBookSystemTest: Verify that total person status is updated

(📚 Archive) (📈 Interdiff between v3 and v4) (📈 Range-Diff between v3 and v4)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/803/4/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

pyokagan · 2018-08-09T06:44:20Z

Seems like adjustments for the following commits were made:

9eac635 (Refactor codebase to use java.nio.file for file/path handling, 2018-04-21)
1b4a6a7 (DarkTheme.css: update styleClass to stack-pane, 2018-04-01)

Would be nice if the other PRs had some kind of similar changelog.

pyokagan · 2018-08-09T06:45:01Z

Would be nice if the other PRs had some kind of similar changelog.

So we can tell if the way the conflicts were resolved are correct or not.

In order to determine the total number of contacts in the address book, the user has to scroll the list all the way down to the last person, as the ID of the last person gives an indication of the total number of persons. This forces the user to use the graphical user interface to find such an information. Let's modify StatusBarFooter so that it shows the total number of persons in the current address book.

The application now shows the total number of persons in the address book on the status bar. Let's modify the system tests to verify the new behavior.

CanIHasReview-bot · 2018-08-18T12:58:40Z

v5

@yamgent submitted v5 for review.

[v5 1/2] StatusBarFooter: Show total persons
[v5 2/2] AddressBookSystemTest: Verify that total person status is updated

(📚 Archive) (📈 Interdiff between v4 and v5) (📈 Range-Diff between v4 and v5)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/803/5/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

yamgent · 2018-08-18T12:58:57Z

This PR had to be rebased again due to conflicts. No other changes otherwise (i.e. no requested changes because there isn't one).

76d8e4e: StatusBarFooterHandle.java - removed the use of this. in both the constructor and setTotalPersons().
fe696a3: AddressBookSystemTest.java - since assertApplicationStartingStateIsCorrect() no longer uses try/catch, had to make sure the new code was like that as well.

CanIHasReview-bot added the s.ToReview label Jan 19, 2018

Zhiyuan-Amos reviewed Jan 20, 2018

View reviewed changes

yamgent force-pushed the getting-started-ui-status branch 2 times, most recently from e1e7303 to 57e442c Compare January 20, 2018 03:03

Zhiyuan-Amos added the pr.Answer Pull request is answer for a tutorial in developer guide. Do not actually merge. label Jan 20, 2018

pyokagan added s.Ongoing and removed s.ToReview labels Jan 21, 2018

Zhiyuan-Amos mentioned this pull request Jan 24, 2018

Rebase PRs used in suggested programming tasks #784

Closed

yamgent force-pushed the getting-started-ui-status branch 7 times, most recently from 26ffce8 to 509eb2d Compare February 9, 2018 03:56

CanIHasReview-bot added s.ToReview and removed s.Ongoing labels Feb 9, 2018

se-edu deleted a comment from yamgent Feb 9, 2018

Zhiyuan-Amos reviewed Feb 9, 2018

View reviewed changes

Zhiyuan-Amos added s.Ongoing and removed s.ToReview labels Feb 9, 2018

yamgent force-pushed the getting-started-ui-status branch from 509eb2d to c761df2 Compare February 11, 2018 03:23

CanIHasReview-bot added s.ToReview and removed s.Ongoing labels Feb 11, 2018

Zhiyuan-Amos requested a review from damithc February 11, 2018 08:36

damithc approved these changes Feb 11, 2018

View reviewed changes

Zhiyuan-Amos removed the s.ToReview label Feb 11, 2018

yamgent mentioned this pull request Aug 4, 2018

Rebase all answer PRs #891

Closed

7 tasks

yamgent added the s.Ongoing label Aug 9, 2018

yamgent force-pushed the getting-started-ui-status branch from c761df2 to 65bef93 Compare August 9, 2018 05:46

CanIHasReview-bot added s.ToReview and removed s.Ongoing labels Aug 9, 2018

yamgent requested a review from a team August 9, 2018 05:59

pyokagan approved these changes Aug 9, 2018

View reviewed changes

yamgent added s.Ongoing and removed s.ToReview labels Aug 18, 2018

yamgent force-pushed the getting-started-ui-status branch from 65bef93 to fe696a3 Compare August 18, 2018 12:37

yamgent added 2 commits August 18, 2018 20:50

AddressBookSystemTest: Verify that total person status is updated

c4ab246

The application now shows the total number of persons in the address book on the status bar. Let's modify the system tests to verify the new behavior.

yamgent force-pushed the getting-started-ui-status branch from fe696a3 to c4ab246 Compare August 18, 2018 12:50

CanIHasReview-bot added s.ToReview and removed s.Ongoing labels Aug 18, 2018

yamgent requested a review from a team August 18, 2018 12:59

Zhiyuan-Amos approved these changes Aug 18, 2018

View reviewed changes

yamgent requested a review from damithc August 19, 2018 02:12

damithc approved these changes Aug 19, 2018

View reviewed changes

yamgent removed the s.ToReview label Aug 19, 2018

xsot mentioned this pull request Feb 19, 2019

Total person count in status bar cs2103-ay1819s2-w11-1/main#4

Merged

damithc closed this Dec 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ui: Show total persons in status bar #803

Ui: Show total persons in status bar #803

yamgent commented Jan 19, 2018 •

edited

Loading

CanIHasReview-bot commented Jan 19, 2018

Zhiyuan-Amos left a comment

Zhiyuan-Amos left a comment

Zhiyuan-Amos Jan 20, 2018

yamgent commented Jan 20, 2018

CanIHasReview-bot commented Feb 9, 2018

Zhiyuan-Amos left a comment

Zhiyuan-Amos Feb 9, 2018

Zhiyuan-Amos Feb 9, 2018

Zhiyuan-Amos Feb 9, 2018

CanIHasReview-bot commented Feb 11, 2018

CanIHasReview-bot commented Aug 9, 2018

pyokagan commented Aug 9, 2018

pyokagan commented Aug 9, 2018

CanIHasReview-bot commented Aug 18, 2018

yamgent commented Aug 18, 2018 •

edited

Loading

Ui: Show total persons in status bar #803

Ui: Show total persons in status bar #803

Conversation

yamgent commented Jan 19, 2018 • edited Loading

CanIHasReview-bot commented Jan 19, 2018

v1

Zhiyuan-Amos left a comment

Choose a reason for hiding this comment

Zhiyuan-Amos left a comment

Choose a reason for hiding this comment

Zhiyuan-Amos Jan 20, 2018

Choose a reason for hiding this comment

yamgent commented Jan 20, 2018

CanIHasReview-bot commented Feb 9, 2018

v2

Zhiyuan-Amos left a comment

Choose a reason for hiding this comment

Zhiyuan-Amos Feb 9, 2018

Choose a reason for hiding this comment

Zhiyuan-Amos Feb 9, 2018

Choose a reason for hiding this comment

Zhiyuan-Amos Feb 9, 2018

Choose a reason for hiding this comment

CanIHasReview-bot commented Feb 11, 2018

v3

CanIHasReview-bot commented Aug 9, 2018

v4

pyokagan commented Aug 9, 2018

pyokagan commented Aug 9, 2018

CanIHasReview-bot commented Aug 18, 2018

v5

yamgent commented Aug 18, 2018 • edited Loading

yamgent commented Jan 19, 2018 •

edited

Loading

yamgent commented Aug 18, 2018 •

edited

Loading