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

Update UG #148

Merged
merged 9 commits into from
Oct 27, 2020
Merged

Conversation

andreatanky
Copy link

@andreatanky andreatanky commented Oct 25, 2020

No description provided.

@andreatanky andreatanky added this to the v1.3b milestone Oct 25, 2020
@andreatanky andreatanky self-assigned this Oct 25, 2020
@codecov-io
Copy link

codecov-io commented Oct 25, 2020

Codecov Report

Merging #148 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #148      +/-   ##
============================================
- Coverage     52.07%   52.06%   -0.02%     
- Complexity      559      560       +1     
============================================
  Files           113      114       +1     
  Lines          2452     2468      +16     
  Branches        301      301              
============================================
+ Hits           1277     1285       +8     
- Misses         1093     1100       +7     
- Partials         82       83       +1     
Impacted Files Coverage Δ Complexity Δ
src/main/java/seedu/address/ui/AssignmentCard.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...eedu/address/logic/parser/DeleteCommandParser.java 100.00% <0.00%> (ø) 2.00% <0.00%> (-2.00%)
...eedu/address/logic/parser/RemindCommandParser.java 100.00% <0.00%> (ø) 2.00% <0.00%> (ø%)
...ava/seedu/address/logic/commands/CommandLogic.java 84.61% <0.00%> (ø) 9.00% <0.00%> (?%)
...va/seedu/address/logic/commands/RemindCommand.java 87.87% <0.00%> (+2.16%) 10.00% <0.00%> (ø%)
...in/java/seedu/address/logic/parser/ParserUtil.java 74.00% <0.00%> (+2.57%) 15.00% <0.00%> (+2.00%)
...va/seedu/address/logic/commands/DeleteCommand.java 100.00% <0.00%> (+7.14%) 7.00% <0.00%> (-8.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b1e31d...c1d2e3f. Read the comment docs.

Copy link
Collaborator

@ChooJiaXin ChooJiaXin 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 :) Minor nits on phrasing but otherwise looks good. There seems to be something wrong with the formatting on GitHub though, so do take note.

Comment on lines 142 to 144
You can use `NUMBER_OF_DAYS` index to quickly view assignments that you need to complete soon! (with deadlines nearing).
For example, `list 2` will show you assignments that are due 2 days (48 hours) from the current date (and current time).
{: .alert .alert-gitlab-purple}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The phrasing for line 142 sounds a little strange. Maybe "... quickly view assignments that are due soon." would be better?

Copy link
Author

Choose a reason for hiding this comment

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

Thought "you need to complete soon" sounds a bit more impactful :')

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, then maybe removing the (with deadlines nearing) would be better in terms of phrasing :)

Copy link
Collaborator

@ChooJiaXin ChooJiaXin left a comment

Choose a reason for hiding this comment

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

Left some comments below :)

Comment on lines 166 to 185
Here is the table of prefixes that is used:
<style type="text/css">
.tg {border-collapse:collapse;border-spacing:0;}
.tg td{border-color:black;border-style:solid;border-width:1px;font-family:Arial, sans-serif;font-size:14px;
overflow:hidden;padding:10px 5px;word-break:normal;}
.tg th{border-color:black;border-style:solid;border-width:1px;font-family:Arial, sans-serif;font-size:14px;
font-weight:normal;overflow:hidden;padding:10px 5px;word-break:normal;}
.tg .tg-0lax{text-align:left;vertical-align:top}
</style>
<table class="tg">
<thead>
<tr>
<th class="tg-0lax">Prefix</th>
<th class="tg-0lax">Type of keyword</th>
<th class="tg-0lax">Syntax with examples</th>
<th class="tg-0lax">Remarks</th>
</tr>
</thead>
<tbody>
<tr>
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this table, I don't think the "type of keyword" column is very meaningful as the syntax column already mentions the type of keyword in all caps. Also, personally, I feel that the "syntax with example" column is a little cluttered. Maybe separating that column into 2 columns would be better? (one syntax column and one examples column)

Comment on lines 211 to 214

Format: `find PREFIX/ KEYWORD [MORE KEYWORDS]`


Copy link
Collaborator

Choose a reason for hiding this comment

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

Format line should be placed right after the "Finding assignments" subheader to preserve consistency :)

Comment on lines 216 to 218
Parameters are:
- n/ NAME_OF_ASSIGNMENT [MORE NAME_OF_ASSIGNMENTS] to find by name of assignment.
- d/ DATE_OR_TIME_OF_ASSIGNMENT [MORE DATE_OR_TIME_OF_ASSIGNMENT] to find by the deadline (date or time) of assignment.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently, the parameters section and the table appear to have some slight overlap. You may consider either incorporating the parameters section into the "Remarks" column in the table, or edit the parameters section such that it is distinct from the table.

Copy link
Collaborator

@hyngkng hyngkng 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 the current format is great! Added my comments below :)


--------------------------------------------------------------------------------------------------------------------
## About
This user guide provides you with the necessary information on how to become an expert user of ProductiveNUS.
Before moving on to the next section, [Getting started](#getting-started), you can familiarize yourself with the terminologies, syntax and icons used in this user guide by reading the following sub-sections.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think it's better to remove the "Before moving on to the ..." and go straight to "You can familiarise yourself ..."? The first part seems a little redundant to me. Unless you want to prevent users from "skipping ahead" of the user guide :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, remember to change to "familiarise"

Comment on lines +121 to +125
You can list all your assignments with `list`. Alternatively, you can type `list` followed by an index `NUMBER_OF_DAYS` to list your assignments with deadlines that fall within the current date (and time) and `NUMBER_OF_DAYS` later (in number of hours).

The `NUMBER_OF_DAYS` in hours is multiplied by 24.

For example, `list 3` lists all your assignments that are due within 3 days (72 hours) from the current date (and current time). If the current date and time is 24/10/2020 12:00 pm, all assignments due from this date and time to 27/10/2020 12:00PM will be displayed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can omit the multiplied by 24 hours information as the users do not need to know. Just telling them the input number refers to the number of days should suffice :)

Comment on lines +136 to +137
- `list`
- `list 7`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, this might be redundant but perhaps adding "lists all your assignments" and "lists all your upcoming assignments in the coming week" or something similar might be a nice touch!

@andreatanky andreatanky merged commit d3b1407 into AY2021S1-CS2103T-F11-3:master Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants