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

Specify editorFonts w/ UIFont & not KanvasTextFont #15

Merged
merged 3 commits into from
Feb 8, 2021
Merged

Conversation

bjtitus
Copy link
Collaborator

@bjtitus bjtitus commented Oct 28, 2020

Replaces the KanvasTextFont type with UIFonts.

This allows us to add fonts without having to change the library to handle stringly typed names.

The analytics provider will now be responsible for converting the UIFont to a string.

Clients implementing KanvasCameraAnalyticsProvider will need to do any conversion from the UIFont to an appropriate string for logging. In the example the font's name is used, but a mapping like the previous KanvasTextFont provided can also be used.

This allows us to add fonts without having to change the library to handle stringly typed names.
The analytics provider will now be responsible for converting the UIFont to a string.
@bjtitus bjtitus self-assigned this Oct 28, 2020
@bjtitus bjtitus marked this pull request as ready for review October 28, 2020 16:05
@bjtitus bjtitus requested review from jschementi and removed request for jschementi October 28, 2020 16:41
@bjtitus
Copy link
Collaborator Author

bjtitus commented Oct 28, 2020

@jschementi Not sure if you want to wait until after we release 1.0 (so that there are no changes needed to Orangina) for these changes which alter the public API.

@jschementi
Copy link
Contributor

I'd hold off on any public API changes until Orangina is locked to a specific version of Kanvas. So, after launch.

Copy link
Collaborator

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

:shipit:

@bjtitus bjtitus merged commit de99a16 into main Feb 8, 2021
@bjtitus bjtitus deleted the stories/fonts branch February 8, 2021 16:57
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

3 participants