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

[RNMobile] - Layout picker - Update emoji #21373

Merged
merged 2 commits into from
Apr 8, 2020

Conversation

geriux
Copy link
Member

@geriux geriux commented Apr 3, 2020

Gutenberg Mobile PR -> wordpress-mobile/gutenberg-mobile#2103

Description

While testing on Android, an issue was spotted on devices with API 21 and 22 where the emoji wouldn't render properly because we were using some from a newer Unicode version. So we need to update it to make it compatible with those. While we give support to that range of OS versions we need to pick emojis from the Unicode 6.0 definition

How has this been tested?

  • Create an Android Emulator with API 21 and 22
  • Open the app with metro running
  • Create a page
  • Expect to see the Layout picker buttons visible and all emojis should render properly

Screenshots

Before Services emoji After Services emoji
iOS Contact emoji Android Contact emoji

Types of changes

Bug fix

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.

@geriux
Copy link
Member Author

geriux commented Apr 3, 2020

While testing I spotted the "Envelope" -> ✉️ emoji wasn't working correctly for API Level >= 21 <=23 as well.

Screenshot_1585757038

I tested going to the Unicode Version 1.0 from the emulator's browser and barely some rendered correctly from that list, then I checked the 6.0 version and almost all of them rendered correctly. @iamthomasbishop is there another emoji we could use as a replacement for the "Envelope" (✉️) emoji?

@github-actions
Copy link

github-actions bot commented Apr 3, 2020

Size Change: +1.29 kB (0%)

Total Size: 885 kB

Filename Size Change
build/block-directory/index.js 6.03 kB -1 B
build/block-editor/index.js 102 kB +127 B (0%)
build/block-editor/style-rtl.css 10.6 kB -115 B (1%)
build/block-editor/style.css 10.6 kB -118 B (1%)
build/blocks/index.js 57.5 kB -1 B
build/components/index.js 195 kB -4 B (0%)
build/edit-navigation/index.js 2.71 kB +228 B (8%) 🔍
build/edit-post/index.js 92.5 kB +229 B (0%)
build/edit-post/style-rtl.css 12.1 kB +63 B (0%)
build/edit-post/style.css 12.1 kB +64 B (0%)
build/edit-site/index.js 9.78 kB +667 B (6%) 🔍
build/edit-site/style-rtl.css 4.68 kB +70 B (1%)
build/edit-site/style.css 4.68 kB +71 B (1%)
build/editor/index.js 42.8 kB -3 B (0%)
build/element/index.js 4.44 kB +1 B
build/format-library/index.js 6.95 kB +1 B
build/i18n/index.js 3.57 kB +1 B
build/notices/index.js 1.57 kB +1 B
build/priority-queue/index.js 789 B +9 B (1%)
build/redux-routine/index.js 2.84 kB +1 B
build/warning/index.js 1.14 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.45 kB 0 B
build/api-fetch/index.js 3.8 kB 0 B
build/autop/index.js 2.59 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.53 kB 0 B
build/block-library/style.css 7.54 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-navigation/style-rtl.css 239 B 0 B
build/edit-navigation/style.css 241 B 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.74 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.6 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@iamthomasbishop
Copy link

iamthomasbishop commented Apr 3, 2020

@geriux For the contact one, I think we could use either “email”, “closed mailbox with raised flag”, or “memo” from the 6.0 spec.

@geriux
Copy link
Member Author

geriux commented Apr 3, 2020

Thanks for the options @iamthomasbishop here's how they'd look:

E-Mail

iOS Android

Closed Mailbox with Raised Flag

iOS Android

Memo

iOS Android

Which one do you prefer? All three render correctly 😃

@iamthomasbishop
Copy link

Which one do you prefer?

I think I prefer the mailbox one, but all 3 of them work just fine.

@geriux geriux added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Apr 3, 2020
@geriux geriux requested review from mkevins and chipsnyder April 3, 2020 14:12
@geriux geriux marked this pull request as ready for review April 3, 2020 14:12
Copy link
Contributor

@chipsnyder chipsnyder left a comment

Choose a reason for hiding this comment

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

Tested on API 24 and 27 the 🔧(fix) still looks good on those. Once Matt checks I say 📫(Ship it)

I also did a spot check on iOS 11.0.1 and everything looks good there too

Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

Fyi: I had to merge with latest develop on gutenberg-mobile to resolve an interface mismatch with WordPress-Android develop.

After that:

Looks good, tested via emulator on API 21 and 23 👍 . Nice work, LGTM!

@geriux geriux merged commit 8fc02a0 into master Apr 8, 2020
@geriux geriux deleted the rnmobile/fix-layout-picker-emoji branch April 8, 2020 07:03
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 8, 2020
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) [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants