Repository navigation
Conversation
|
Hi ! |
81df683 to
8f5469f
Compare
|
@qnga Hey! :)
Sure! Thanks for fixing the issue.
I agree that Readium already has a good system to build font families through In our app Android XML font family resources are the source of truth. We use them across XML layouts and Compose. The app is white-label, with 100+ flavors, and each flavor can override resources such as strings, colors, font families, etc. This follows Android’s built in approach for defining font families in resources, as described here. The XML file groups the font files together with their style and weight details, and exposes them as a single font resource. Without XML font family support in Readium we need to duplicate the same font family definition in
Our app could parse these XML files itself and convert them to Readium declarations but then every app with the same need would have to implement its own parser/bridge. Since this is a standard Android resource format and maps directly to Readium’s existing This PR does not replace Readium’s font family system. It only adds a bridge from Android |
|
Sorry for the delay. I feel like the Android XML font family resources are more or less deprecated as I can't find a way to use them in Jetpack Compose. If I'm not mistaken, I'll feel more comfortable if we could keep all of this simple supporting only simple font resources. |
|
Hey! Thanks for the reply. I agree that XML font family resources will probably become less common in favor of Compose font families. At the same time, storing font files ( Currently, Readium only supports loading custom app provided fonts from the I agree that including an entire XML parser in Readium adds unnecessary complexity. As an alternative, I could remove the XML parser from this PR and keep only the support for loading fonts from Android resources. The Internally, Would this approach be acceptable? For context, I'm currently using a somewhat hacky workaround in my project. I have a custom |
Yep, that's what I meant. |
|
I removed XML parsing from the PR 👍 |
| // For the custom font Literata. | ||
| "fonts/.*", | ||
| // For Roboto Mono font files in Android resources | ||
| EpubNavigatorFragment.androidResourcePath("font/roboto_mono_.*"), |
There was a problem hiding this comment.
Too tricky. Let's be both clear and safe with two different lists: servedAssets et servedAndroidResources.
| val resources = requireContext().resources | ||
|
|
||
| val robotoMonoRegular = requireNotNull( | ||
| EpubNavigatorFragment.androidResourceUrl(resources, R.font.roboto_mono_regular) | ||
| ) | ||
|
|
||
| val robotoMonoItalic = requireNotNull( | ||
| EpubNavigatorFragment.androidResourceUrl(resources, R.font.roboto_mono_italic) | ||
| ) | ||
|
|
||
| val robotoMonoBold = requireNotNull( | ||
| EpubNavigatorFragment.androidResourceUrl(resources, R.font.roboto_mono_bold) | ||
| ) | ||
|
|
||
| val robotoMonoBoldItalic = requireNotNull( | ||
| EpubNavigatorFragment.androidResourceUrl(resources, R.font.roboto_mono_bold_italic) | ||
| ) |
There was a problem hiding this comment.
These declarations would be clearer below, with their uses.
| * `font/my_font.ttf`. Simple glob patterns such as `font/my_font_.*` | ||
| * can be used with [Configuration.servedAssets]. | ||
| */ | ||
| public fun androidResourcePath(path: String): String = |
| * | ||
| * @return `null` if [resId] does not identify a file-backed resource. | ||
| */ | ||
| public fun androidResourcePath(resources: Resources, resId: Int): String? = |
There was a problem hiding this comment.
I'm not sure if this needs to be public. Returning a URL is best, isn't it?
| WebViewAssetLoader.Builder() | ||
| .setDomain(ASSETS_HOSTNAME) | ||
| .addPathHandler("/$ANDROID_RES_PATH_PREFIX/", WebViewAssetLoader.ResourcesPathHandler(application)) | ||
| .addPathHandler("/", WebViewAssetLoader.AssetsPathHandler(application)) |
There was a problem hiding this comment.
Let's be super explicit and add a path prefix for regular assets too (things could get messy if there is an android_res directory in regular assets). Please check the hard-coded asset URLs in the code base.
There was a problem hiding this comment.
Or maybe a different hostname for resources if that's easier? I don't have a strong opinion on this.
Related issue: #786
Android allows resources to be provided using either the
resorassetsfolders. Classic Android apps provide most resources, such as images, strings, and fonts, usingresmaking them easy to use in XML layouts, views, and composables.Currently, Readium allows custom fonts to be provided only through the
assetsfolder. This means apps need to store the same font in bothresandassetsto use it in XML and in Readium.This PR removes that limitation and allows the font family to be specified using either a single font file or an Android XML font family.
Summary
@font-facedeclarations.Changes
Core navigator support
readium/navigator/src/main/java/org/readium/r2/navigator/epub/resources/ResourcePaths.ktreadium/navigator/src/main/java/org/readium/r2/navigator/epub/resources/ResourceFontFamily.ktConfiguration.addFontFamilyDeclaration(...)readium/navigator/src/main/java/org/readium/r2/navigator/epub/resources/FontFamilyDeclarations.ktWebView serving updates