Text Editor: Support custom styles

[aforcier]

Closes #185. Adds a button to the text editor which cycles through six text styles. The text styles each have a particular typeface, along with rules like text size, line spacing, letter spacing, and shadow.

Remaining TODOs

General

• AddedView implementation
• Consider serializing to portable units -> Deferred until text feature is done
• Look into fallback fonts for other scripts, probably create a new issue -> Text Editing: Support fallback fonts #498, basically Not-MVP
• Test in WPAndroid in case of any theming issues

PR feedback

• Add fallback mechanism when downloadable fonts aren't available

Design feedback

• Shrink font button and text to align with surrounding image buttons
• Capitalize sentences in text editor
• Reduce scrim overlay by 10%
• Adjust scrim overlay further pending feedback
• Adjust line height for Oswald, maybe others
• Support smart quotes/apostrophes -> Text Editing: Support smart quotes #503, Not-MVP
• Decrease line height for Space Mono and Shrikhand
• Increase letterspacing for Shrikhand
• Decrease shadow length for Shrikhand
• Hide text in background while it's being edited
• Fix clipping of text on the left for certain fonts and letters -> Text Editing: Fix clipping of text on the left for certain fonts and letters #508, will fix in its own PR
• Add padding to added TextViews so line breaks are kept in the same place as they were in the editor

Implementation notes

Font loading

I went with Android downloadable fonts, which allows us to save on apk size considerably by letting Android asynchronously download the fonts (or not if they already exist on the device because another app downloaded them already) instead of bundling them in our apk. There is one exception, Oswald, which I added directly as a ttf file:

Oswald

The designs called for Oswald to be uppercase. There isn't a variation of the font like this, so I looked into a few approaches. A simple solution is to change the input filter to all caps, and turn the text uppercase. We wouldn't want to permanently make all the text uppercase though, since someone just cycling through fonts would have to re-write their text. I started to back up the text and restore it after cycling through Oswald, which works but it's still not clear what should happen when changes are made to the text while Oswald is selected.

I looked at some other apps - some seem to do what I did, but some were clearly not modifying the input filter because the keyboard wasn't forced to all caps. One possible way they might have done this would be to have a custom EditText implementation that has separate 'inputted text' and 'displayed text'. This isn't trivial though, and we'd need to keep track of this when handing the text off to be added as a TextView in the canvas, and back to being edited.

Instead, I opened Oswald in a font glyph editor and created a modified version where all the lowercase glyphs were replaced with their uppercase versions.

So for example, the glyph for U+0061, which is lowercase a, is mapped to a glyph that looks like A.

This way we can avoid any custom uppercase logic in the code, and all text entered is respected without us having to do anything, because the underlying code points are maintained.

The cost is that we have to include the font file in our apk, which is ~100KB. Since it's the only one that feels okay for now, we could even reduce the size by removing Cyrillic support which is included in Oswald, but is pretty much the only non-Latin Extended script any of our custom fonts support.

IdentifiableTypeface

Stories does some text serializing/deserializing since anything we created in the text editor is replicated in a TextView, and back. This isn't a problem for most text styling (size, letter spacing, color, etc), but is a problem for typefaces. The reason is that once you set a view's Typeface, there's no way I could fine to reverse lookup that back into a font ID, so we can't tell what rules go with that typeface, and we can't even serialize it.

I solved this by using a custom TextView which deprecates the usual typeface methods and instead supports a custom IdentifiableTypeface class, which is just a wrapper for Typeface with a unique ID.

TextStyler and FontResolver

We're starting to pass a lot of style parameters back and forth between PhotoEditor and ComposeLoopFrameActivity. I decided to encapsulate this in a TextStyler class that can be passed around more easily. Since PhotoEditor probably shouldn't know what custom fonts we support, I'm also using a FontResolver interface to let the stories module tell PhotoEditor which typeface is associated with which ID.

To test

1. Add text
2. Push button
3. Repeat 😀

But also,

1. Check that all style attributes are kept when going between the editor and the canvas
2. Turn on Don't Keep Activities and confirm that all style information on the text is preserved
3. Test on earlier APIs

[aforcier]

Ready for review! A few notes:

1. Per discussion with @mzorz we'll hold off changing the serialization format until the text editing feature is done, then we can update Stories projects: interoperability discussion #477 / Stories projects interoperability: strip Android specifics #478 with the full set of data we're storing and make a central decision/update.
2. There's some pending design feedback - hold off on actually merging for now, and I'll either update the PR with design changes or open a new PR.

[mzorz]

This is so cool @aforcier!
Playing with the feature is delightful. Thanks for bringing this cool stuff in! 🙌🏻

I looked at some other apps - some seem to do what I did, but some were clearly not modifying the input filter because the keyboard wasn't forced to all caps. One possible way they might have done this would be to have a custom EditText implementation that has separate 'inputted text' and 'displayed text'. This isn't trivial though, and we'd need to keep track of this when handing the text off to be added as a TextView in the canvas, and back to being edited.

Instead, I opened Oswald in a font glyph editor and created a modified version where all the lowercase glyphs were replaced with their uppercase versions.

So for example, the glyph for U+0061, which is lowercase a, is mapped to a glyph that looks like A.

This way we can avoid any custom uppercase logic in the code, and all text entered is respected without us having to do anything, because the underlying code points are maintained.

This is interesting. Also, we've learned from past experiences in Aztec that using InputFilter is problematic wordpress-mobile/AztecEditor-Android#580 when users have a soft keyboard other than the device's setup (i.e. SwitfKey or other apps), so I think your approach is probably the best path forward too.

The cost is that we have to include the font file in our apk, which is ~100KB. Since it's the only one that feels okay for now, we could even reduce the size by removing Cyrillic support which is included in Oswald, but is pretty much the only non-Latin Extended script any of our custom fonts support.

I think it's fine - we should keep Cyrillic if it's already supported. I don't know much about fonts but I wonder if there exists a upper case / lower case mode for Cyrillic we may be concerned about? For example in German nouns always start with upper case, and it's considered a misspelling not to do so. Wonder if such a thing like "having all caps is a misspelling" exists in any language 🤔 - probably a nitpick.

IdentifiableTypeface

I solved this by using a custom TextView which deprecates the usual typeface methods and instead supports a custom IdentifiableTypeface class, which is just a wrapper for Typeface with a unique ID.

I think it's good - as we were discussed elsewhere, we'll need an univocal way to identify this, which also comes as a strong need between devices / platforms, so I think it's good this came up earlier.

TextStyler and FontResolver

These are neat!

Gave it a spin, these are my notes.

Fallback mechanism if font not present

I wondered what if downloading the fonts go wrong? Can we still use the app? What's the fallback mechanism? I nice thing to try might be to install the app on a new (preferably older) emulator with airplane mode ON.

Effectively tried installing this on a Pixel 3XL API 29 emulator with airplane mode ON, the app launched (logs show it failed to retrieve the fonts) and then when I tried to add text it crashed, here we have the logs:

2020-08-17 09:48:15.222 2895-5096/com.google.android.gms I/FontsContentProvider: Received query name=Nunito&weight=700, URI content://com.google.android.gms.fonts
2020-08-17 09:48:15.223 2895-5096/com.google.android.gms I/FontsContentProvider: Query [name=Nunito&weight=700] resolved to {Nunito, wdth 100.0, wght 700, ital 0.0, bestEffort false}
2020-08-17 09:48:15.245 1791-29965/? W/EmuHWC2: validate: layer 217 CompositionType 4, fallback
2020-08-17 09:48:15.253 2895-18885/com.google.android.gms E/FontsHttpClientDL: Download https://fonts.gstatic.com/s/a/443daf06190f4c7abd94b7917adb41656e6014e81c12eb036656d63f2b0c08a3.ttf failed
2020-08-17 09:48:15.253 2895-23756/com.google.android.gms E/FontFetch: https://fonts.gstatic.com/s/a/443daf06190f4c7abd94b7917adb41656e6014e81c12eb036656d63f2b0c08a3.ttf Status{statusCode=Unable to resolve host "fonts.gstatic.com": No address associated with hostname, resolution=null}; declaring failure
2020-08-17 09:48:15.254 2895-5096/com.google.android.gms I/FontsContentProvider: Fetch {Nunito, wdth 100.0, wght 700, ital 0.0, bestEffort false} end status Status{statusCode=Unable to resolve host "fonts.gstatic.com": No address associated with hostname, resolution=null}
2020-08-17 09:48:15.269 1791-29965/? W/EmuHWC2: validate: layer 217 CompositionType 4, fallback
2020-08-17 09:48:15.269 2895-5096/com.google.android.gms I/FontsContentProvider: Pulling font file for id = 0, cache size = 5
2020-08-17 09:48:15.270 29531-29531/com.automattic.loop D/AndroidRuntime: Shutting down VM
2020-08-17 09:48:15.271 29531-29531/com.automattic.loop E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.automattic.loop, PID: 29531
    android.content.res.Resources$NotFoundException: Font resource ID #0x7f090001 could not be retrieved.
        at androidx.core.content.res.ResourcesCompat.loadFont(ResourcesCompat.java:365)
        at androidx.core.content.res.ResourcesCompat.getFont(ResourcesCompat.java:230)
        at com.wordpress.stories.compose.text.TextStyleGroupManager.getFont(TextStyleGroupManager.kt:100)
        at com.wordpress.stories.compose.text.TextStyleGroupManager.<init>(TextStyleGroupManager.kt:47)
        at com.wordpress.stories.compose.text.TextEditorDialogFragment.onAttach(TextEditorDialogFragment.kt:41)
        at androidx.fragment.app.Fragment.performAttach(Fragment.java:2672)
        at androidx.fragment.app.FragmentStateManager.attach(FragmentStateManager.java:263)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1170)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1356)
        at androidx.fragment.app.FragmentManager.moveFragmentToExpectedState(FragmentManager.java:1434)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1497)
        at androidx.fragment.app.BackStackRecord.executeOps(BackStackRecord.java:447)
        at androidx.fragment.app.FragmentManager.executeOps(FragmentManager.java:2169)
        at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1992)
        at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1947)
        at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1849)
        at androidx.fragment.app.FragmentManager$4.run(FragmentManager.java:413)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

Also curious note: just turning airplane mode back OFF didn't automatically trigger the font downloading process (not that I found anything that says so, but that was my expectation somehow). Waited a few minutes and nothing interesting appeared in the logs. However, it was fetched when I first wanted to access the feature (when I tapped the text control on the slide composer), when hitting ResourcesCompat.getFont(context, fontRes) in TextStyleGroupManager this time:

2020-08-17 09:56:24.074 2895-5096/com.google.android.gms I/FontsContentProvider: Received query name=Nunito&weight=700, URI content://com.google.android.gms.fonts
2020-08-17 09:56:24.074 2895-5096/com.google.android.gms I/FontsContentProvider: Query [name=Nunito&weight=700] resolved to {Nunito, wdth 100.0, wght 700, ital 0.0, bestEffort false}

[...]


2020-08-17 09:56:24.512 2895-5096/com.google.android.gms I/FontsContentProvider: Fetch {Nunito, wdth 100.0, wght 700, ital 0.0, bestEffort false} end status Status{statusCode=SUCCESS, resolution=null}
2020-08-17 09:56:24.517 2895-5096/com.google.android.gms I/FontsContentProvider: Pulling font file for id = 52, cache size = 4
2020-08-17 09:56:24.531 2895-5096/com.google.android.gms I/FontsContentProvider: Received query Libre Baskerville, URI content://com.google.android.gms.fonts
2020-08-17 09:56:24.531 2895-5096/com.google.android.gms I/FontsContentProvider: Query [Libre Baskerville] resolved to {Libre Baskerville, wdth 100.0, wght 400, ital 0.0, bestEffort false}
2020-08-17 09:56:24.577 2895-5096/com.google.android.gms I/FontsContentProvider: Fetch {Libre Baskerville, wdth 100.0, wght 400, ital 0.0, bestEffort false} end status Status{statusCode=SUCCESS, resolution=null}
2020-08-17 09:56:24.580 2895-4158/com.google.android.gms I/FontsContentProvider: Pulling font file for id = 53, cache size = 4
2020-08-17 09:56:24.588 2895-4158/com.google.android.gms I/FontsContentProvider: Received query Pacifico, URI content://com.google.android.gms.fonts
2020-08-17 09:56:24.588 2895-4158/com.google.android.gms I/FontsContentProvider: Query [Pacifico] resolved to {Pacifico, wdth 100.0, wght 400, ital 0.0, bestEffort false}

[...]

I wonder if this could have any impact on slow connections (given these are downloaded and shown right there.

We should probably do something to wait for fonts to be fully loaded, or have a fallback mechanism if font is not present. The fallback mechanism should include reflecting the availability of fonts on the selector, ideally? (for example, by not showing all of them, or only showing the default font, or just not showing the selector at all).

EmojiCompat

I wondered about things like how does this play with EmojiCompat? What we have now is that we run the initEmojiCompat() and issue the FontRequest upon apps's start, and then on each time we use addText or addEmoji we're going through the emojiCompat callback to make sure to parse it out correctly.

I verified this will still work because of how EmojiCompat works (https://developer.android.com/guide/topics/ui/look-and-feel/emoji-compat) -> the library identifies emoji for a given CharSequence, replaces them with EmojiSpans, and then renders the glyphs from the passed font (using Noto Color Emoji Compat as per the docs to be compatible https://www.google.com/get/noto/help/emoji/)

Other thoughts / notes

• seen you declared the fonts declaration in the manifest, which is neat because the host app (demo or WPAndroid) will get this via manifesto merger. Wondered whether we could do the same for EmojiCompat, but realized we still need the initialization code to run ASAP on the host app, so moving the font request font-family resource files would be just a tiny enhancement (we'd still need the initialization code for it to be complete)
  As a further enhancement, we could perhaps do something like encapsulating that code in some other class within the stories library, and requesting that that method gets called from the Application class.

• this is from the alignment feature but, realized I didn't comment on this before. When you enter some text and play with the text center tool, somehow tapping on the center tool sets an expectation about the output for me (i.e. alligning to the right I expect the text to appear there), but then if the text was short enough the output is not really changed (it makes sense when you know what happens behind the scenes). Wonder if that may be perceived by users as confusing or "not working" as well.

Will now go through code and leave notes there if anything 👍

[mzorz]

TIL https://kotlinlang.org/docs/reference/annotations.html and everything related.

[aforcier]

This is interesting. Also, we've learned from past experiences in Aztec that using InputFilter is problematic wordpress-mobile/AztecEditor-Android#580 when users have a soft keyboard other than the device's setup (i.e. SwitfKey or other apps), so I think your approach is probably the best path forward too.

Ah yes - I felt uneasy as soon as I even typed InputFilter, I think I may have ran into issues with it in the past as well (the visual editor maybe). Yeah I think that's a whole box of fun bugs we get to avoid. 😀

I think it's fine - we should keep Cyrillic if it's already supported. I don't know much about fonts but I wonder if there exists a upper case / lower case mode for Cyrillic we may be concerned about? For example in German nouns always start with upper case, and it's considered a misspelling not to do so. Wonder if such a thing like "having all caps is a misspelling" exists in any language 🤔 - probably a nitpick.

From my own basic knowledge and from asking around/researching, I don't think there's an issue there. I mapped all lowercase Cyrillic characters to uppercase in the font and it's pretty common for all-caps to be used, at least in Russian. If things are different in another Cyrillic-using language, that's a pretty out there edge case we can deal with if anyone ever brings up an issue with it I'd say.

Effectively tried installing this on a Pixel 3XL API 29 emulator with airplane mode ON, the app launched (logs show it failed to retrieve the fonts) and then when I tried to add text it crashed

We should probably do something to wait for fonts to be fully loaded, or have a fallback mechanism if font is not present. The fallback mechanism should include reflecting the availability of fonts on the selector, ideally? (for example, by not showing all of them, or only showing the default font, or just not showing the selector at all).

Ouf, nice catch there. I have an idea for how to approach this I'll try out and update on.

verified this will still work because of how EmojiCompat works (https://developer.android.com/guide/topics/ui/look-and-feel/emoji-compat) -> the library identifies emoji for a given CharSequence, replaces them with EmojiSpans, and then renders the glyphs from the passed font (using Noto Color Emoji Compat as per the docs to be compatible https://www.google.com/get/noto/help/emoji/)

Awesome, thanks for verifying that - I did check that emoji were working but I didn't dive into the EmojiCompat aspect. 👍

[aforcier]

seen you declared the fonts declaration in the manifest, which is neat because the host app (demo or WPAndroid) will get this via manifesto merger. Wondered whether we could do the same for EmojiCompat, but realized we still need the initialization code to run ASAP on the host app, so moving the font request font-family resource files would be just a tiny enhancement (we'd still need the initialization code for it to be complete)
As a further enhancement, we could perhaps do something like encapsulating that code in some other class within the stories library, and requesting that that method gets called from the Application class.

Right - I remember I was a bit thrown here when I started working on the feature because I realized the EmojiCompat declaration and download logic was in the app module, so for a moment I thought we were missing it in WPAndroid. Then I saw you had ported it over and implemented it in the WordPress app class. I think it would be better to encapsulate that in the stories library and call it from WPAndroid like you mentioned, but it's a small improvement we can get to later (and who knows this might actually be important elsewhere in WPAndroid, so it may as well be centrally defined).

this is from the alignment feature but, realized I didn't comment on this before. When you enter some text and play with the text center tool, somehow tapping on the center tool sets an expectation about the output for me (i.e. alligning to the right I expect the text to appear there), but then if the text was short enough the output is not really changed (it makes sense when you know what happens behind the scenes). Wonder if that may be perceived by users as confusing or "not working" as well.

I don't think this is a huge deal but I see how it might feel like a bug. cc @megsfulton do you think we should change the behavior?

[aforcier]

Updated the TODO list with the fallback issue and some fresh design feedback from Megs, working through the list.

[aforcier]

@mzorz this is ready for a second pass 🎉

Added a fallback method for fonts in case we're not able to download the custom ones (21b2b65)

Here we'll basically fall back to the default font and request the closest style we can to the original (i.e. Serif, Sans Serif, Monospace, Default Bold). The rest of the styles (spacing, shadow, size) are still applied.

If the fonts are downloaded later, editing an existing text in fallback style will apply the proper font.

I ran this fallback behavior by Megs and it sounded good to her 👍

Now hiding the text from the background while editing (8d871c1)

Megs pointed out that it's pretty distracting to write over a second copy of the text when editing text that was already added to the view.

I see we were already hiding the TextView when it was empty, I modified things to always hide the currently editing view.

Issue with clipping (not to be resolved in this PR)

This one's a bit trickier than I initially thought. I have an approach in mind but it's going to affect text background color too. I opened #508 and I'll address the issue probably next sprint when I have an approach working for background color I can test against.

Assorted style tweaks (not applied in this PR yet)

I sent @megsfulton a test APK with some of the style changes that are still unchecked in the list - I'll update the PR when we've settled those values. Hold off merging until then please @mzorz.

[mzorz]

Now hiding the text from the background while editing (8d871c1)
Megs pointed out that it's pretty distracting to write over a second copy of the text when editing text that was already added to the view.

I see we were already hiding the TextView when it was empty, I modified things to always hide the currently editing view.

Cool! This would close this issue here then #434 👍

Added a fallback method for fonts in case we're not able to download the custom ones (21b2b65)
Here we'll basically fall back to the default font and request the closest style we can to the original (i.e. Serif, Sans Serif, Monospace, Default Bold). The rest of the styles (spacing, shadow, size) are still applied.

If the fonts are downloaded later, editing an existing text in fallback style will apply the proper font.

Awesome! Nice you found a way to make it more robust 👍 😄

Issue with clipping (not to be resolved in this PR)
This one's a bit trickier than I initially thought. I have an approach in mind but it's going to affect text background color too. I opened #508 and I'll address the issue probably next sprint when I have an approach working for background color I can test against.

Sounds good - doesn't sound like a blocker; wonder how bad the problem is? Let me know if I can help 🙇

Assorted style tweaks (not applied in this PR yet)
I sent @megsfulton a test APK with some of the style changes that are still unchecked in the list - I'll update the PR when we've settled those values. Hold off merging until then please @mzorz.

Sure thing!

Nice work on this second pass as well @aforcier thank you!

[aforcier]

Okay @mzorz , ready for round 3! 🎉 Changes:

• Small style changes to Space Mono and Shrikhand, @megsfulton gave the 👍

• Added the same padding to added text views as I added to the editor:

Megs pointed out what was basically a regression on #180 - the line breaks in the added text view didn't always match what was shown in the editor.

The cause was that I added some padding to the editor text in #466 that needed to be reflected in the added text view. This was done in a2ac933, but I wanted to mention that it also fixes part of the text clipping issue (#508).

You can check this by using the 'Playful' font before and after the patch, and starting new lines with lowercase p and j.
You'll notice things look fine in the editor, but the letters are clipped on the left a bit when added to the view. This shouldn't happen anymore after checking out the tip of the branch.

This still leaves some more text clipping issues, I think they're mostly just related to the Bold style. I have a fix in mind which I'll get to with the background color work as I mentioned.

[mzorz]

As said before this is delightful!

• verified the regression of Text line breaks don't always match result #180 is fixed, and saw how it contributes to solving Text Editing: Fix clipping of text on the left for certain fonts and letters #508 partially
• found a minor issue (not a blocker) with selected text start and end caret positions not moving along (or being reset) when the alignment changes, we can either fix it here if you think it's an easy fix or we can open an issue (not worth spending much time, especially if it's more of an Android thing after all) 👍

Let me know what you think and feel free to merge 👍

Thanks a lot this is beautiful! 😄 🎉

[aforcier]

found a minor issue (not a blocker) with selected text start and end caret positions

Oh interesting one - I don't have any quick ideas, we're not doing anything super special. I suppose we might need to re-draw the view (and probably re-apply the selection)? That solution seems like it might be janky though and worse than the actual problem. 🤔

I don't think it's a huge deal (the problem is visual only, the actual selection is correctly preserved as far as I can tell), so I opened #510 and we can revisit it later.

Thanks a lot this is beautiful! 😄 🎉

🙇 props once more to @megsfulton for designing this thing of beauty 😀