-
Notifications
You must be signed in to change notification settings - Fork 411
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
[CS2103T-W13-2] Njoy #137
base: master
Are you sure you want to change the base?
[CS2103T-W13-2] Njoy #137
Conversation
*Step 2*. The method `Model#addQuestion(Question question)` will then be called to add the question and a success message will | ||
be generated by the `QuestionAddCommand#generateSuccessMessage(Question question)` method and a new `CommandResult` will be | ||
returned with the generated success message. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be a sequence diagram explaining the control flows for the add/edit/delete/list commands.
Currently, you only have steps in bullet points, but it's unclear which objects are involved and which are active at what times.
docs/DeveloperGuide.adoc
Outdated
Below is the sequence diagram of the interactions that happen from when the slideshow command is entered, to the corresponding | ||
questions displayed in the slideshow. | ||
|
||
image::SlideshowFeatureSequenceDiagram.png[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice diagram! It could use a bit more exposition on what happens to the new QuestionPanel
s. Are they added to the UI scene graph anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this diagram is quite large. Would it be worth breaking it up into two diagrams, by using ref
and sd
blocks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Logic part of the UI? Would be clearer if it uses different colour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to include a caption for the diagram
docs/DeveloperGuide.adoc
Outdated
it falls below `0`. The behaviour of this follows the common procedure that most presentation programs adopt thus, it will not | ||
feel foreign to users. | ||
|
||
image::SlideshowFeatureActivityDiagram.png[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the navigation loop, how is the current question incremented? As of now, your action box only mentions navigation, but does not specify directly how the current question counter is affected.
One possibility is to add another action box indicating the user incrementing/decrementing the question index
*** [.big]##Question Number## | ||
*** Question Topic & Options | ||
*** [.small]#Answer# | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are the questions/answers represented in the UI structure/scene graph? Is there a particular UiPart handling each question? How are they created and organized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good job!
@@ -14,7 +14,7 @@ ifdef::env-github[] | |||
endif::[] | |||
:repoURL: https://github.com/se-edu/addressbook-level3/tree/master | |||
|
|||
By: `Team SE-EDU` Since: `Jun 2016` Licence: `MIT` | |||
By: `Team AY1920S1-CS2103T-W13-2` Since: `September 2019` Licence: `MIT` | |||
|
|||
== Setting up | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design should be updated away from the base AB3 context.
docs/DeveloperGuide.adoc
Outdated
The commands when executed, will interface with the methods exposed by the `Model` interface to perform the related operations | ||
(See <<Design-Logic, logic component>> for the general overview). | ||
|
||
_To Add: Class diagram of the interaction between the question parser and commands_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking forward to see the diagram!
docs/DeveloperGuide.adoc
Outdated
Since it is optional for the users to input fields, the fields not entered will reuse the existing value currently defined in | ||
the `Question` object. | ||
[NOTE] | ||
If the question type is changed from open ended to mcq, it is necessary for the user to defined all four options i.e a/ b/ c/ d/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps for clarity and to avoid confusion, state OpenEndedQuestion
instead of "open ended" and McqQuestion
instead of "mcq"?
docs/DeveloperGuide.adoc
Outdated
@@ -235,13 +403,74 @@ image::CommitActivityDiagram.png[] | |||
** Cons: Requires dealing with commands that have already been undone: We must remember to skip these commands. Violates Single Responsibility Principle and Separation of Concerns as `HistoryManager` now needs to do two different things. | |||
// end::undoredo[] | |||
|
|||
=== Generating Statistics Feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this section is not shown in your netlify preview. You might want to look into it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
docs/DeveloperGuide.adoc
Outdated
Below is the sequence diagram of the interactions that happen from when the slideshow command is entered, to the corresponding | ||
questions displayed in the slideshow. | ||
|
||
image::SlideshowFeatureSequenceDiagram.png[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Logic part of the UI? Would be clearer if it uses different colour.
docs/DeveloperGuide.adoc
Outdated
Below is the sequence diagram of the interactions that happen from when the slideshow command is entered, to the corresponding | ||
questions displayed in the slideshow. | ||
|
||
image::SlideshowFeatureSequenceDiagram.png[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to include a caption for the diagram
Group/Student/Tag/Mark command tests
…w command section
UG/DG cleanup amd UML diagrams added.
Updated PPP with details
* upstream/master: HTML EOF updated PPP Fix Ui.png
Update PPP, UG and DG
Remove duplicate Ui.png file and update PPP
# Conflicts: # README.adoc
Adjust PPP, DG Caption formatting, Add quiz export to command summary and application help guide
UG and DG update
HTML PPP and ADOC changes
@fabbbbbbyy
@alages97
@jeongyh99
@lumwb