[RNMobile] Final Page Templates with localization by koke · Pull Request #19525 · WordPress/gutenberg

koke · 2020-01-09T11:29:01Z

Waiting for #19106, rebase to master before it's ready for final review.

Description

This seems like the most sensible approach to template i18n for now. It would add some friction to creating and maintaining the templates, but we could create some tooling around that so we can create templates in Gutenberg, then export a JSON. It needs some comments for translators to clarify that it’s example content, but other than that this might be it

For wordpress-mobile/gutenberg-mobile#1473

How has this been tested?

Open the main app with metro running
Go to pages
Create a new page
Select a template
Expect: to see the modal with the preview
Tap on Apply
Expect: the content of the preview to be in the editor and the modal should close

Screenshots

About	Contact

Portfolio	Services

Team	Team

Types of changes

Adds final set of page templates for mobile with their localization

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.
I've updated all React Native files affected by any refactorings/renamings in this PR. .

pinarol · 2020-01-10T16:27:11Z

It would be good to add more description about how to generate the json of the page in a practical way.

…template-i18n # Conflicts: # packages/block-editor/src/components/page-template-picker/default-templates.js

geriux · 2020-02-13T10:37:45Z

Hey @iamthomasbishop ! Can you please review these emojis? I chose some random ones for Services and Team 😅

geriux · 2020-02-13T10:59:34Z

@koke I can't assign you as a reviewer because it's your PR 😄but whenever you have time, could you please check the added templates? I think they have the right format. Thanks!

iamthomasbishop · 2020-02-13T20:41:48Z

Hey @iamthomasbishop ! Can you please review these emojis? I chose some random ones for Services and Team 😅

Definitely! And we should probably start a spreadsheet to document which emoji/icon each layout uses 😄 Here's a start:

Layout Name	Emoji name	Emoji	Info
Services	Hammer and wrench	🛠	https://emojipedia.org/hammer-and-wrench/
Team	Busts in silhouette	👥	https://emojipedia.org/busts-in-silhouette/
About	Waving hand	👋	https://emojipedia.org/waving-hand/
Contact	Envelope	✉️	https://emojipedia.org/envelope/
Blog	Newspaper	📰	https://emojipedia.org/newspaper/
Portfolio	Artist palette	🎨	https://emojipedia.org/artist-palette/

I'm not 100% sure if it matches what you've already got, feel free to correct me 😄

geriux · 2020-02-14T08:56:17Z

Thank you @iamthomasbishop !

Updated them:

Icons	Icons

koke · 2020-02-14T13:05:56Z

+							'https://a8ctm1.files.wordpress.com/2019/08/scatter-1.jpg?w=640',
+						link:
+							'https://a8ctm1.wordpress.com/portfolio/scatter-3/',
+						id: '658',


This could be problematic. I imagine those IDs are for the site where the templates were created, but it could break things on any other site.

Also perhaps we should avoid the links as week, it's one thing that we are hot linking images from a demo site, but having them link to that site as well would be unexpected and confusing.

Interestingly enough, if I remove the links and ids from the template, when you try to add an image to that gallery, it replaces all the existing images. I'm not entirely sure if that's good or bad 😁

😅You're right, I've just tested that myself. Were there any discussions about these cases? Blocks that have media content: Gallery, Media & Text etc.?

Not that I'm aware of, I think we didn't consider it could be an issue. I'd say removing the IDs and links is likely the least harmful option. It might be a bit confusing if they disappear when the user adds a new image, but they are meant to be placeholders, so maybe not that confusing? cc @pinarol @iamthomasbishop

Why don't we loose the IDs but keep the links? When we insert an image from a web url we don't have any ID at all, so it should be a valid case?

 <figure class="wp-block-image size-large"><img src="https://a8ctm1.files.wordpress.com/2019/08/scatter-1.jpg" alt=""/></figure> 

Why don't we loose the IDs but keep the links? When we insert an image from a web url we don't have any ID at all, so it should be a valid case?

This works! Forgot about linking images directly by URL, so there won't be any issues if we remove the IDs

Interestingly enough, if I remove the links and ids from the template, when you try to add an image to that gallery, it replaces all the existing images.

This looks like a bug? It should be happening independent from the SPTs

Hmm I don't know if its a bug or just an edge case. I tried to do that on the Web editor and the gallery removes the images when you add a new one as well. (Using this template without IDs)

Ok thanks for trying on web.

So do we have a consensus about moving fwd with only links? @koke @geriux Anything left to prevent merging this PR?

All good from my side =)

Looking good to me too

iamthomasbishop · 2020-02-18T02:41:54Z

Looking good, @geriux!

koke · 2020-02-20T10:37:58Z

I can't approve since I'm the original author but from me. I tested yarn genstrings and the new strings where there in the translation files.

geriux · 2020-02-20T12:15:05Z

I can't approve since I'm the original author but from me. I tested yarn genstrings and the new strings where there in the translation files.

Thanks!!

koke added Internationalization (i18n) Issues or PRs related to internationalization efforts Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Jan 9, 2020

koke force-pushed the add/spt-template-i18n branch from 87213dc to df44ae6 Compare January 24, 2020 11:14

koke changed the base branch from add/spt-template-preview to master January 24, 2020 11:14

Initial i18n support for page templates

da3dee5

koke force-pushed the add/spt-template-i18n branch from df44ae6 to da3dee5 Compare January 24, 2020 11:16

Gerardo Pacheco added 2 commits February 13, 2020 09:09

Merge branch 'master' of github.com:WordPress/gutenberg into add/spt-…

904ccad

…template-i18n # Conflicts: # packages/block-editor/src/components/page-template-picker/default-templates.js

RNMobile - Added templates with localization

2793d8a

geriux mentioned this pull request Feb 13, 2020

Final Page Templates with localization wordpress-mobile/gutenberg-mobile#1897

Merged

2 tasks

geriux changed the title ~~Page Template localization~~ [RNMobile] Final Page Templates with localization Feb 13, 2020

geriux marked this pull request as ready for review February 13, 2020 10:34

geriux requested review from ellatrix and youknowriad as code owners February 13, 2020 10:34

pinarol reviewed Feb 13, 2020

View reviewed changes

Comment thread packages/block-editor/src/components/page-template-picker/default-templates.js Outdated

RNMobile - Page templates - Split pages in files

550537d

RNMobile - Page templates - Icons updated

33b613c

koke commented Feb 14, 2020

View reviewed changes

RNMobile - Page templates - Remove IDs and links from media

8a80af4

geriux approved these changes Feb 20, 2020

View reviewed changes

geriux merged commit 5d8701c into master Feb 20, 2020

geriux deleted the add/spt-template-i18n branch February 20, 2020 12:55

github-actions Bot added this to the Gutenberg 7.6 milestone Feb 20, 2020

Conversation

koke commented Jan 9, 2020 • edited by geriux Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

Uh oh!

pinarol commented Jan 10, 2020

Uh oh!

geriux commented Feb 13, 2020

Uh oh!

geriux commented Feb 13, 2020

Uh oh!

Uh oh!

iamthomasbishop commented Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geriux commented Feb 14, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamthomasbishop commented Feb 18, 2020

Uh oh!

koke commented Feb 20, 2020

Uh oh!

geriux commented Feb 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

koke commented Jan 9, 2020 •

edited by geriux

Loading

iamthomasbishop commented Feb 13, 2020 •

edited

Loading