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

Implemenet initial draft of UG #166

Merged
merged 11 commits into from
Oct 28, 2020

Conversation

AaronnSeah
Copy link

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #166 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #166   +/-   ##
=========================================
  Coverage     72.25%   72.25%           
  Complexity     1321     1321           
=========================================
  Files           181      181           
  Lines          4448     4448           
  Branches        627      627           
=========================================
  Hits           3214     3214           
  Misses         1014     1014           
  Partials        220      220           

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 8895acc...f938c8d. Read the comment docs.

Copy link

@sebastiantoh sebastiantoh left a comment

Choose a reason for hiding this comment

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

LGTM, left with some nitpicky formatting stuff

docs/UserGuide.md Outdated Show resolved Hide resolved
1. type `contact find alx yo` in the command box and press `Enter`.
<img src="images/contactfindfirststep.png" alt="result for 'contact sort keyword'" width="400px">
2. the contact list now contains contacts whose name is similar to alx yo.
<img src="images/contactfindsecondstep.png" alt="result for 'contact sort keyword'" width="400px">

Choose a reason for hiding this comment

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

I took a look at your images, and I'm not sure why your aspect ratio is different from mine. Mine is a rectangle which is longer horizontally than it is vertically. Maybe we can check with the rest how their apps look, and standardise it? Otherwise our screenshots will look very inconsistent. After which, we can decide on a width that we should all follow for images. 400px looks too small for mine. So right now, most of my images are set to 900px

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the check! I will try to fix my image settings to look like yours asap

docs/UserGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Outdated Show resolved Hide resolved
Aaron S and others added 2 commits October 28, 2020 22:05
Co-authored-by: sebastiantoh <sebas.tsj.98@gmail.com>
Copy link

@Asthenosphere Asthenosphere left a comment

Choose a reason for hiding this comment

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

Other than Sebastian's comments, I think it would be better if can remove the top menu of the window so that the UI looks consistent regardless of the OSes.

AaronnSeah and others added 5 commits October 28, 2020 22:06
Co-authored-by: sebastiantoh <sebas.tsj.98@gmail.com>
Co-authored-by: sebastiantoh <sebas.tsj.98@gmail.com>
Co-authored-by: sebastiantoh <sebas.tsj.98@gmail.com>
Co-authored-by: sebastiantoh <sebas.tsj.98@gmail.com>
Co-authored-by: sebastiantoh <sebas.tsj.98@gmail.com>
@AaronnSeah
Copy link
Author

thanks @Asthenosphere , I will crop those out, but there are some images with a new window though I think it will be hard to crop the header of the second window

@Asthenosphere Asthenosphere merged commit f1bb8a9 into AY2021S1-CS2103T-T11-1:master Oct 28, 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