Skip to content

Allow configuration of default fonts used in TextView#1221

Merged
SergioEstevao merged 10 commits intodevelopfrom
issue/allow_change_of_default_font
Oct 9, 2019
Merged

Allow configuration of default fonts used in TextView#1221
SergioEstevao merged 10 commits intodevelopfrom
issue/allow_change_of_default_font

Conversation

@SergioEstevao
Copy link
Copy Markdown
Contributor

@SergioEstevao SergioEstevao commented Oct 8, 2019

This PR brings to TextView the following:

  • The possibility to replace the defaultFont being used after the TextView is created
  • Introduction of a FontProvider to allow configuration of fonts being used in the system
  • Configuration of the default font being used for monospace

To test:

  • Run the sample app
  • Check the demo content
  • There is new button on the top right (+) press it and see if the default font changes in all the demo content.

@SergioEstevao
Allow dynamic change of default font.
1f47ac8
@SergioEstevao
Remove unused code.
efc0760
@SergioEstevao
Use font provider class to provide default fonts
cdb2814 
Instead of having random places where we obtain/configure fonts, this
commits creates a central class where fonts can be retrieved.
@SergioEstevao
Make font properties public
b155e7d
@SergioEstevao SergioEstevao added this to the Next Stable milestone Oct 8, 2019
@SergioEstevao SergioEstevao requested a review from etoledom October 8, 2019 20:58
etoledom
etoledom reviewed Oct 9, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Thank you @SergioEstevao for this Improvement!

I tested as described and it works well.
But I noticed that the font changes in all the content that doesn't have any styling. Bold, Italic, heading, etc keeps its current font.

I did a test with a more obvious font:

Is this expected?
font

This is Zapfino font. But it doesn't have Bold or Italic variant.
Also tested with Optima font that does have Bold and Italic variant and the result was the same.

One more thing: When I tested with Zapfino I got a crash here:

AztecEditor-iOS/Aztec/Classes/TextKit/LayoutManager.swift

Line 321 in f9ff2d3

return UIFont(descriptor: descriptor!, size: font.pointSize)

It was quick fix by replacing the force unwrap with an if-let. But does it make sense that the will be nil given that font and traits are not nil? 😬

@SergioEstevao
Copy link
Copy Markdown
Contributor Author

SergioEstevao commented Oct 9, 2019

@etoledom code updated ready for another look.

etoledom
etoledom approved these changes Oct 9, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Thank you for the update @SergioEstevao !
It's working great now 🎉

I left a code comment to think about, but everything looks great, so ✅

@SergioEstevao SergioEstevao merged commit 181ebc7 into develop Oct 9, 2019
@SergioEstevao SergioEstevao deleted the issue/allow_change_of_default_font branch October 9, 2019 11:45
@etoledom etoledom mentioned this pull request Oct 14, 2019
Merged
@SergioEstevao SergioEstevao modified the milestones: Next Stable, 1.11.0 Nov 21, 2019
@SergioEstevao SergioEstevao mentioned this pull request Jan 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@etoledom etoledom etoledom approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Type] Enhancement

Projects

None yet

Milestone

1.11.0

Development

Successfully merging this pull request may close these issues.

2 participants

@SergioEstevao @etoledom