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.

Sign up for GitHub

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

Jump to bottom

[RNMobile] Do not use hard coded fontFamily in Image block #13677

Merged
merged 21 commits into from
Feb 6, 2019
Merged

[RNMobile] Do not use hard coded fontFamily in Image block #13677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
00ec3ca
Use RichText component in Title block for mobile.
daniloercoli Jan 28, 2019
45f8bde
Fix lint
daniloercoli Jan 28, 2019
68b353c
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
daniloercoli Jan 30, 2019
f332203
Set font family, weight, and size via RN props for Title, Heading, an…
daniloercoli Jan 30, 2019
63586f1
Fix lint
daniloercoli Jan 30, 2019
06d9f84
Adds font* props to PlainText for mobile
daniloercoli Jan 31, 2019
5a43ea9
Make `serif` default family for RichText on mobile
daniloercoli Jan 31, 2019
3f8f97c
Set the correct font family for Image caption on mobile
daniloercoli Jan 31, 2019
bd629d8
Set the correct font family for Nextpage block on mobile
daniloercoli Jan 31, 2019
624b473
Set the correct font family for Code block on mobile
daniloercoli Jan 31, 2019
10ad47d
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
daniloercoli Feb 1, 2019
63346cf
set the correct font family for Image caption on mobile
daniloercoli Feb 1, 2019
c029cb4
Remove extra fontFamily props.
daniloercoli Feb 1, 2019
8b52a1a
Remaps some font names so that they work in iOS. (#13628)
diegoreymendez Feb 1, 2019
2a126ab
Make sure PlainText takes in consideration the fontFamily passed via …
daniloercoli Feb 1, 2019
70836f1
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
daniloercoli Feb 5, 2019
544f096
Do not use hard coded `fontFamily` value, instead use CSS style.
daniloercoli Feb 5, 2019
d9dba31
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
daniloercoli Feb 5, 2019
3ff3d9c
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
daniloercoli Feb 5, 2019
db0c608
Add missing new line at end of style
daniloercoli Feb 5, 2019
398c26e
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
daniloercoli Feb 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/block-library/src/image/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export default class ImageEdit extends React.Component {
<View style={ { padding: 12, flex: 1 } }>
<TextInput
style={ { textAlign: 'center' } }
fontFamily={ 'serif' }
fontFamily={ this.props.fontFamily || ( styles[ 'caption-text' ].fontFamily ) }
underlineColorAndroid="transparent"
value={ caption }
placeholder={ __( 'Write caption…' ) }
Expand Down
8 changes: 8 additions & 0 deletions packages/block-library/src/image/styles.native.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// @format

@import "variables.scss";

.imageContainer {
flex: 1;
justify-content: center;
Expand All @@ -16,6 +20,10 @@
align-items: center;
}

.caption-text {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: for simplicity in accessing it, and to match the rest of the class definitions in the same file, what do you think about renaming it to .captionText?

Copy link
Contributor Author

@daniloercoli daniloercoli Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, captionText was my 1st choice yesterday, but got an error in JS lint if I rename it:

[0] gutenberg-mobile/gutenberg/packages/block-library/src/image/edit.native.js
[0]   294:57  error  ["captionText"] is better written in dot notation  dot-notation

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha, apparently the rules weren't in place in the past and the other class definitions had passed. Oh well, caption-text it is then :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that message is not about the dash, but it's telling you to replace styles['captionText'] with styles.captionText

font-family: $default-regular-font;
}

.resetSettingsButton {
color: $alert-red;
}