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

Remaps some font names so that they work in iOS. #13628

Merged

Conversation

diegoreymendez
Copy link
Contributor

@diegoreymendez diegoreymendez commented Feb 1, 2019

Description

Remaps the font names for native so that they also work in iOS.

How has this been tested?

Tested through: wordpress-mobile/gutenberg-mobile#530

Types of changes

Changes some font names in the native .js files to also work in iOS.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@diegoreymendez diegoreymendez added the Mobile Web Viewport sizes for mobile and tablet devices label Feb 1, 2019
@gziolo gziolo added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) and removed Mobile Web Viewport sizes for mobile and tablet devices labels Feb 1, 2019
@daniloercoli
Copy link
Contributor

daniloercoli commented Feb 1, 2019

Unfortunately these changes don't work fine on Android. Both Courier and NotoSerif aren't available on Android.

The following GIST here: https://gist.github.com/daniloercoli/14b811c39eb74abfe9ca0b6d2ba2c5e4 is a viable solution to the problem.

@hypest
Copy link
Contributor

hypest commented Feb 1, 2019

Can we use SCSS styles for this? We can have .ios.scss and .android.scss variants to differentiate the platforms and it doesn't have to be a runtime check at all.

@daniloercoli
Copy link
Contributor

Font related properties need to be passed as props to the component.
We can use the CSS file variants where we set them via CSS.

@diegoreymendez
Copy link
Contributor Author

diegoreymendez commented Feb 1, 2019

@hypest - That's a good observation, but I think it may be better to hold on that for now, let me explain why.

Ultimately the component shouldn't define its own font... in our case the fonts we set should really be an app-level definition.

So I was hoping, right after we finish this, to make sure (through an extra PR) that the fonts are passed to these components by our gutenberg-mobile JavaScript code, probably through props (and from core CSS files). The thing is, I've been avoiding optimizations for now, since any extra files and code added at the component level is something we'll have to also delete when we move to the proposed solution.

All in all, I was hoping to scope these PRs to just making the fonts be right, and afterwards spend some time in the proposal above.

How does that sound?

@hypest
Copy link
Contributor

hypest commented Feb 1, 2019

Hmm, maybe I have missed some context here, sorry if so.

Seems to me that this PR just changes one hardcoded value to another. And as per Danilo's comment the change doesn't work on Android. So, a cross-platform solution is needed anyway.

With that as a starting point, it seems that we can use the style files already in place per platform (see the Code block's SCSS files for android and here) and we can have similar for any block that doesn't already.

We also have in place a way to go at the "app level" and set variables that can be reused by any SCSS file. See the _variables.scss file. If that doesn't apply well, we can even introduce a new, dedicated for mobile global variables file and set it here in the transformer on the gutenberg-mobile side.

It's more of a code-structure thing at this point and less an optimization point.

What am I missing? 🤔

@daniloercoli
Copy link
Contributor

daniloercoli commented Feb 1, 2019

What am I missing? 🤔

We didn't find a way yet to pass fontFamily to AztecWrapper via CSS. The only solution that works fine with our wrapper is to use props.

I guess we will try to move to CSS in another PR.

@diegoreymendez
Copy link
Contributor Author

@daniloercoli - This is ready for review, but please don't merge your PR to develop yet,as I have to do the actual work of making sure the title looks good in iOS.

@daniloercoli daniloercoli self-requested a review February 1, 2019 17:32
Copy link
Contributor

@daniloercoli daniloercoli left a comment

Choose a reason for hiding this comment

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

Nice work, really!

@daniloercoli daniloercoli merged commit 8b52a1a into rnmobile/372-use-RichText-on-Title-block Feb 1, 2019
@daniloercoli daniloercoli deleted the rnmobile/improve-372 branch February 1, 2019 17:33
diegoreymendez pushed a commit that referenced this pull request Feb 5, 2019
* Use RichText component in Title block for mobile.
This is required to properly intercpet Enter.key on all platforms/keyboards. We decided to move to RichText since all of the work for Enter.key intercept was laready done there per Para and Heading blocks.

* Fix lint

* Set font family, weight, and size via RN props for Title, Heading, and Para blocks.

* Fix lint

* Adds font* props to PlainText for mobile

* Make `serif` default family for RichText  on mobile

* Set the correct font family for Image caption on mobile

* Set the correct font family for Nextpage block on mobile

* Set the correct font family for Code block on mobile

* set the correct font family for Image caption on mobile

* Remove extra fontFamily props.

* Remaps some font names so that they work in iOS. (#13628)

* Remaps some font names so that they work in iOS.

* Improved the logic for picking the default font in some components.

* Modifies the logic that sets the default font for the code component.

* Changes the font family in a css file.

* Adds an import to have a default font for native.

* Standardizes the default font for the code component.

* Simplifies the default font for the code block.

* Simplifies the default font for the code block.

* Configures the default font for plain-text from a css file.

* Fixes the styling for the rich text components and restores a line that was removed by mistake.

* Fixes a linting problem.

* Fixes some linting issues.

* Fixes a linting issue.

* Make sure PlainText takes in consideration the fontFamily passed via props before falling back to default styles
daniloercoli added a commit that referenced this pull request Feb 6, 2019
* Use RichText component in Title block for mobile.
This is required to properly intercpet Enter.key on all platforms/keyboards. We decided to move to RichText since all of the work for Enter.key intercept was laready done there per Para and Heading blocks.

* Fix lint

* Set font family, weight, and size via RN props for Title, Heading, and Para blocks.

* Fix lint

* Adds font* props to PlainText for mobile

* Make `serif` default family for RichText  on mobile

* Set the correct font family for Image caption on mobile

* Set the correct font family for Nextpage block on mobile

* Set the correct font family for Code block on mobile

* set the correct font family for Image caption on mobile

* Remove extra fontFamily props.

* Remaps some font names so that they work in iOS. (#13628)

* Remaps some font names so that they work in iOS.

* Improved the logic for picking the default font in some components.

* Modifies the logic that sets the default font for the code component.

* Changes the font family in a css file.

* Adds an import to have a default font for native.

* Standardizes the default font for the code component.

* Simplifies the default font for the code block.

* Simplifies the default font for the code block.

* Configures the default font for plain-text from a css file.

* Fixes the styling for the rich text components and restores a line that was removed by mistake.

* Fixes a linting problem.

* Fixes some linting issues.

* Fixes a linting issue.

* Make sure PlainText takes in consideration the fontFamily passed via props before falling back to default styles

* Do not use hard coded `fontFamily` value, instead use CSS style.

* Add missing new line at end of style
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Use RichText component in Title block for mobile.
This is required to properly intercpet Enter.key on all platforms/keyboards. We decided to move to RichText since all of the work for Enter.key intercept was laready done there per Para and Heading blocks.

* Fix lint

* Set font family, weight, and size via RN props for Title, Heading, and Para blocks.

* Fix lint

* Adds font* props to PlainText for mobile

* Make `serif` default family for RichText  on mobile

* Set the correct font family for Image caption on mobile

* Set the correct font family for Nextpage block on mobile

* Set the correct font family for Code block on mobile

* set the correct font family for Image caption on mobile

* Remove extra fontFamily props.

* Remaps some font names so that they work in iOS. (#13628)

* Remaps some font names so that they work in iOS.

* Improved the logic for picking the default font in some components.

* Modifies the logic that sets the default font for the code component.

* Changes the font family in a css file.

* Adds an import to have a default font for native.

* Standardizes the default font for the code component.

* Simplifies the default font for the code block.

* Simplifies the default font for the code block.

* Configures the default font for plain-text from a css file.

* Fixes the styling for the rich text components and restores a line that was removed by mistake.

* Fixes a linting problem.

* Fixes some linting issues.

* Fixes a linting issue.

* Make sure PlainText takes in consideration the fontFamily passed via props before falling back to default styles
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Use RichText component in Title block for mobile.
This is required to properly intercpet Enter.key on all platforms/keyboards. We decided to move to RichText since all of the work for Enter.key intercept was laready done there per Para and Heading blocks.

* Fix lint

* Set font family, weight, and size via RN props for Title, Heading, and Para blocks.

* Fix lint

* Adds font* props to PlainText for mobile

* Make `serif` default family for RichText  on mobile

* Set the correct font family for Image caption on mobile

* Set the correct font family for Nextpage block on mobile

* Set the correct font family for Code block on mobile

* set the correct font family for Image caption on mobile

* Remove extra fontFamily props.

* Remaps some font names so that they work in iOS. (#13628)

* Remaps some font names so that they work in iOS.

* Improved the logic for picking the default font in some components.

* Modifies the logic that sets the default font for the code component.

* Changes the font family in a css file.

* Adds an import to have a default font for native.

* Standardizes the default font for the code component.

* Simplifies the default font for the code block.

* Simplifies the default font for the code block.

* Configures the default font for plain-text from a css file.

* Fixes the styling for the rich text components and restores a line that was removed by mistake.

* Fixes a linting problem.

* Fixes some linting issues.

* Fixes a linting issue.

* Make sure PlainText takes in consideration the fontFamily passed via props before falling back to default styles

* Do not use hard coded `fontFamily` value, instead use CSS style.

* Add missing new line at end of style
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Use RichText component in Title block for mobile.
This is required to properly intercpet Enter.key on all platforms/keyboards. We decided to move to RichText since all of the work for Enter.key intercept was laready done there per Para and Heading blocks.

* Fix lint

* Set font family, weight, and size via RN props for Title, Heading, and Para blocks.

* Fix lint

* Adds font* props to PlainText for mobile

* Make `serif` default family for RichText  on mobile

* Set the correct font family for Image caption on mobile

* Set the correct font family for Nextpage block on mobile

* Set the correct font family for Code block on mobile

* set the correct font family for Image caption on mobile

* Remove extra fontFamily props.

* Remaps some font names so that they work in iOS. (#13628)

* Remaps some font names so that they work in iOS.

* Improved the logic for picking the default font in some components.

* Modifies the logic that sets the default font for the code component.

* Changes the font family in a css file.

* Adds an import to have a default font for native.

* Standardizes the default font for the code component.

* Simplifies the default font for the code block.

* Simplifies the default font for the code block.

* Configures the default font for plain-text from a css file.

* Fixes the styling for the rich text components and restores a line that was removed by mistake.

* Fixes a linting problem.

* Fixes some linting issues.

* Fixes a linting issue.

* Make sure PlainText takes in consideration the fontFamily passed via props before falling back to default styles
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Use RichText component in Title block for mobile.
This is required to properly intercpet Enter.key on all platforms/keyboards. We decided to move to RichText since all of the work for Enter.key intercept was laready done there per Para and Heading blocks.

* Fix lint

* Set font family, weight, and size via RN props for Title, Heading, and Para blocks.

* Fix lint

* Adds font* props to PlainText for mobile

* Make `serif` default family for RichText  on mobile

* Set the correct font family for Image caption on mobile

* Set the correct font family for Nextpage block on mobile

* Set the correct font family for Code block on mobile

* set the correct font family for Image caption on mobile

* Remove extra fontFamily props.

* Remaps some font names so that they work in iOS. (#13628)

* Remaps some font names so that they work in iOS.

* Improved the logic for picking the default font in some components.

* Modifies the logic that sets the default font for the code component.

* Changes the font family in a css file.

* Adds an import to have a default font for native.

* Standardizes the default font for the code component.

* Simplifies the default font for the code block.

* Simplifies the default font for the code block.

* Configures the default font for plain-text from a css file.

* Fixes the styling for the rich text components and restores a line that was removed by mistake.

* Fixes a linting problem.

* Fixes some linting issues.

* Fixes a linting issue.

* Make sure PlainText takes in consideration the fontFamily passed via props before falling back to default styles

* Do not use hard coded `fontFamily` value, instead use CSS style.

* Add missing new line at end of style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants