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

[#214] Fix alignment by changing index font #215

Closed
wants to merge 1 commit into from
Closed

[#214] Fix alignment by changing index font #215

wants to merge 1 commit into from

Conversation

baskargopinath
Copy link
Contributor

@baskargopinath baskargopinath commented Jun 13, 2024

Fixes #214

Explanation below image

image

  • Issue: Alignment problem with index numbers in PersonListCard due to proportional font.
  • Solution: Changed the index font to Arial Mono, a monospaced font.
  • Font Load: Added Arial Mono MT Pro Regular font to the project.
  • CSS Update: Modified cell_big_index style to use Arial Mono.
  • FXML Adjustment: Updated HBox and Label elements for consistent alignment.
  • Outcome: Achieved uniform alignment of index numbers, improving the UI appearance.

Copy link

canihasreview bot commented Jun 13, 2024

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

Copy link

canihasreview bot commented Jun 13, 2024

v1

@baskargopinath submitted v1 for review.

(📚 Archive)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/215/1/head:BRANCHNAME

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

Copy link

canihasreview bot commented Jun 13, 2024

v2

@baskargopinath submitted v2 for review.

(📚 Archive) (📈 Interdiff between v1 and v2) (📈 Range-Diff between v1 and v2)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/215/2/head:BRANCHNAME

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

@damithc
Copy link
Contributor

damithc commented Jun 13, 2024

@baskargopinath If the second commit is simply fixing an error introduced by the first commit, the proper fix is to revise the first commit to not introduce the error at all. That way, we avoid polluting the history with unnecessary commits.

@baskargopinath
Copy link
Contributor Author

Understood prof will fix it asap @damithc

For a non-monospace font such as Segoe UI, "1."
takes up less horizontal space than "2.". This
results in the first name having different
vertical alignment compared to all the names
below the first one in the PersonListCard.

Lets change the font of the index in the
PersonListCard to a monospaced font such as
Arial Mono, which seems to eliminate the
misalignment issue so that the names in the
PersonListCard are aligned.
Copy link

canihasreview bot commented Jun 14, 2024

v3

@baskargopinath submitted v3 for review.

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

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/215/3/head:BRANCHNAME

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

@baskargopinath baskargopinath closed this by deleting the head repository Jun 14, 2024
@damithc
Copy link
Contributor

damithc commented Jun 14, 2024

@baskargopinath two other comments:

  1. It's good enough for the commit message subject to state the WHAT part (no need for the HOW) e.g., Fix misalignment of person names. The HOW part can be in the body.
  2. The max commit message body width is 72 chars. Looks like yours is unnecessarily narrower than that.

@baskargopinath
Copy link
Contributor Author

@damithc okay ived fixed it prof. I had to delete this repo so i can fork AB3-J17 for 2103R. I have sent the PR from my nus github account instead with the requested changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vertically misaligned names in Person List
2 participants