inas0ng / pe Goto Github PK

Typo: reaccessible*

I think the current implementation of findp could pose an issue to users.

Let's say I input "findp al", hoping to find Alice Pauline to edit her details afterwards. And then I backspace in hopes to clear the command input so that I can enter "editp [index]" to edit Alice's details. However, the moment I backspace to "findp a" (the L is removed), the find-query changes and more names appear, potentially changing Alice's index. The only workaround way I can think of is having to use my mouse and click-drag delete the enter command OR press enter, such that the search for "findp al" remains and I can perform "editp [index]" on Alice. This however may not be intuitive since the search is so reactive.

In this case, her index remains at 1 but other cases it may change the index due to differing searches. Hope this is clear :3

Edit: likewise, with findt's implementation

The sequence diagram states that it would return CommandResult(MESSAGE_SUCESS). However, not this command may not always result in success. (i.e., could have no tasks found).

Furthermore, in your code, there is no MESSAGE_SUCESS constant. There is only MESSAGE_TASK_FOUND_S, MESSAGE_TASK_FOUND_P, and MESSAGE_NO_TASK_FOUND.

Similar error may be identified for other sequence diagrams, such as the AddTask sequence diagram which could also result in the MESSAGE_DUPLICATE_TASK instead.

I think maybe to illustrate a successful command call, the team could provide an example, i.e. execute("findt slides"), instead of using execute(commandText).

i typed"findt", expecting it to work like "findp" and immediately list all persons and tasks with the message “Listed all persons and tasks”, however nothing changed.

Hitting enter gives me the error (as it should) stating to enter a keyword behind.

Only way to activate the listing of all persons and tasks is to type "findt " and then delete the trailing space, which seems very unintended. Issue also stems from being inconsistent with "findp" command.

"Prefix and parameters" in blue, looks like it can be clicked but the first instance (first bullet point) is not clickable.

The other instance of "Prefix and parameters" leads me to a Page not Found page on Github.
https://ay2223s2-cs2103-f10-1.github.io/tp/(#12-prefixes-used-in-commands) << this is the faulty link apparently

Understand that this was intended by the team, hence, not a functionality/documentation bug. Just thought that task names may require special characters input in certain cases, could be a potential improvement for future iterations.

The way it's written seems like the help menu automatically closes at the end, which is not the case I believe. Could add a "User closes help menu"

UG states "The local- part should only contain alphanumeric characters and at most one of these special characters, excluding the parentheses, (+_.-)."

however, i can enter Alice's email as [email protected], making use of 2 special characters. This could be a functionality bug or a documentation bug, depending on which is more realistic.

Thought that the use of st/true or st/false could've been improved by using st/done st/notdone to make it more user-friendly. That aside, it was also not clear what true meant and what false meant and this could've been explicitly stated in the UG, i.e. true means done, false means not done.

It is stated in the UG that "The DEADLINE set for tasks must not be before the creation date of the task (i.e you cannot set deadlines in the past)."

This would mean that I should be able to set the deadline of the task to be today, i.e. not before creation date, not in the past. However, setting the deadline as today's date is not allowed.

Edit: ah, i realised that it defaults to 00:00:00! i think it would be more intuitive if it defaulted to 23:59:59 instead :''') could be an area for improvement, if the team agrees

UG states that "Upon entering the findp command,....", however, this takes effect before I hit enter. (could be a phrasing issue or functionality issue)

Also, not sure if this is the best way to implement the command as well since users could've wanted to type editp/addp and accidentally type findp, and before hitting enter, the display has already changed and they might need to relocate the person's index so that they can perform editp/addp.

I liked the implementation of the progress bars and the touch of colour they added! Thought that the colour choice was also very appropriate!

However, I think it's not very clear what the length of the progress bars mean. I was looking at task 2 (the preloaded one) and was wondering for quite awhile why the bar wasn't filled if it was completed. I now assume that the length of the bar indicates how close we are to the deadline? Still unsure as this was not explicitly stated in the UG and hence, can be improved upon! :')

Test

Test

i entered
"filterp tag/marketing" to display benson

entered
"editp 1 tag/marketing" to remove benson's finance tag

benson is edited accordingly (correctly) however no longer displays on the screen. I would still expect benson to show on display due to the filter applied (still has marketing)