-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[GH-473] Port Font to Editor #503
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks almost perfect but maybe also add the tests from Entry that also check the font size and attributes? Namely the FontSizeInitializesCorrectly
and FontAttributesInitializeCorrectly
tests.
Otherwise this is nice and clean and seems to have all the things.
Thanks!
Also, could you add the entry to the sample MainPage. I know this is now almost impossible to test at this point, but once we got some navigation going this will all be much better. |
@mattleibow when you say |
I mean copy the 2 tests from entry to editor |
@mattleibow I see, but when you talk about the MainPage.cs sample, is to add the Editor instead of the entry, right? (Yeah it's a dumb question from me, but just to make sure) |
Ah yeah. Editor. 🤦🏻♀️ So menu words looking the same. |
5885db9
to
2c0ffc4
Compare
src/Core/tests/DeviceTests/Handlers/Editor/EditorHandlerTests.Android.cs
Outdated
Show resolved
Hide resolved
public static void MapFont(EditorHandler handler, IEditor editor) | ||
{ | ||
var services = App.Current?.Services ?? | ||
throw new InvalidOperationException($"Unable to find service provider, the App.Current.Services was null."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related with this PR, but we have this message repeated in several places. Maybe we can create some specific exceptions like FontInitializationException
or group all the messages etc.
Thanks @pictos. Looks nice, just some small changes and I think is ready. |
23d49e9
to
4be75d3
Compare
Description of Change
Implements #473
Additions made
PR Checklist