se-edu / addressbook-level3

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
28 stars 415 forks source link

Revert "Font Issue on M1 Mac Big Sur" #119

Closed kouyk closed 2 years ago

kouyk commented 2 years ago

Reverts se-edu/addressbook-level3#106 to resolve #111.

The original PR aimed to side step the garbled fonts on M1 Mac devices by embedding another font of our choice. One upside and unintended consequence is that fonts are consistent across the various OS regardless of font availability, helping to avoid any potential UI bugs such as text being cut off prematurely etc. However, this does bring about unnecessary complication as more resources are being loaded into the application, such as the one surfaced here, mandating the use of even more workarounds such as the one referenced in #111.

The M1 MacOS issue was eventually resolved through the use of an alternative OpenJDK build that happened to have native M1 binaries as well. Given that the issue originated from OpenJDK rather than AB3 itself, it no longer make sense for the "fix" to be applied to AB3, which seems to cause more trouble than it is worth. Given that most students are fresh to JavaFX, the additional complication does not seem warranted.

canihasreview[bot] commented 2 years ago

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

See this repository's contribution guide for more information.

codecov-commenter commented 2 years ago

Codecov Report

Merging #119 (8971ba4) into master (0f5f654) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #119   +/-   ##
=========================================
  Coverage     72.15%   72.15%           
  Complexity      399      399           
=========================================
  Files            70       70           
  Lines          1232     1232           
  Branches        125      125           
=========================================
  Hits            889      889           
  Misses          311      311           
  Partials         32       32           

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 0f5f654...8971ba4. Read the comment docs.

damithc commented 2 years ago

@kouyk This is an automatic git revert, correct? And this requires us to release a new version, right?

kouyk commented 2 years ago

@kouyk This is an automatic git revert, correct? And this requires us to release a new version, right?

Yes, that's right. I reverted through github, I might need to do some manual fixes to resolve the conflicts now that the help window PR has been merged in.

canihasreview[bot] commented 2 years ago

v1

@kouyk submitted v1 for review.

(:books: Archive)

Checkout this PR version locally ```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/119/1/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.
damithc commented 2 years ago

Thanks very much for the fix @kouyk I'll do a new release