Contact Info Block: changes for adaptation into a mobile block #17123

illusaen · 2020-09-10T01:46:49Z

WIP Design

Fixes #2166

Changes proposed in this Pull Request:

Selecting contact info block instead of Email block on insert.
Changing font size on child blocks to default 16px.
Upping line height on child blocks with 4px padding.
Using Rich Text placeholder colors for dark and light modes.
Pulled Link address to Google Maps toggle into block settings on Address child block.
Changing keyboard type for Email to email keyboard, Phone to phone keypad.
Restricted available blocks to insert when in contact info block to: phone, address, email, separator, heading, and spacer.

Jetpack product discussion

Currently changing behaviours for contact-info for both web and native:

Select contact info block on insert instead of email.
Restricting choices to above choices when inserting child blocks.

Waiting on further dev input to decide on whether to keep web changes or not.

Does this pull request change what data or activity we track or use?

N/A

Testing instructions:

Insert contact info block. Ensure that the block itself is selected instead of any of the children blocks.
Ensure that placeholder text for child blocks are correct color.
Type in text into email field. Ensure that text is correct color.
Repeat for child blocks phone and address.
Select address block.
Select the settings on address block. Ensure that Link to Google maps toggle works and settings block looks correct.
Switch to dark mode.
Repeat steps 2-6 look correct in dark mode.

Proposed changelog entry for your changes:

Changing contact info block behaviour and design to match other blocks.

Screenshots

NOTE: Still working on odd font mismatches between RichText/Contact info blocks and how they seem to differ between light/dark modes and iOS/Android despite having the same settings.

…t-info. Moving google map setting to address block setting.

…ess fragments to use map instead of listing each entry.

…lesFromColorScheme to placeholderTextColor for dark mode. Using Rich Text placeholder text color.

jetpackbot · 2020-09-10T01:55:22Z

Scheduled Jetpack release: November 3, 2020.
Scheduled code freeze: October 27, 2020

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17123

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 39c0de6

etoledom

Thank you for this PR @illusaen, it looks pretty good so far! 🎉

I'd ask you if you could add screenshots with the before-after comparison for @iamthomasbishop to have a look at the changes 🙏
I saw the conversation in the issue ticket but it's good to have a comparison on the PR itself too as documentation :)

About the changes on web, I agree with @iamthomasbishop that we shouldn't change web behaviour. At least no without their review and approval. I think this would be beyond this particular mobile project so we could create an issue for web devs about this when this PR is merged, to get their thoughts about it. How does it sound to you?

I also left a code comment about the keyboard type not working for me. Could you check that out?
Thanks!

extensions/blocks/contact-info/address/edit.native.js

extensions/blocks/contact-info/phone/edit.native.js

…er on contact-info/address/edit in order to remove repeated code.

…for text and text-decoration for light and dark modes. Removing unused code.

etoledom

✅ Selecting contact info block instead of Email block on insert.
✅ Changing font size on child blocks to default 16px.
✅ Upping line height on child blocks with 2px padding. (I think it's 4px in code? Looks good to me though)
✅ Using Rich Text placeholder colors for dark and light modes.
✅ Pulled Link address to Google Maps toggle into block settings on Address child block.
✅ Changing keyboard type for Email to email keyboard, Phone to phone keypad.
✅ Restricted available blocks to insert when in contact info block to: phone, address, email, separator, heading, and spacer.

Everything looks great to me, nice job @illusaen ! 🎉

I left a small suggestion as code comments.
Other than that, there are a couple of lint issues from CI, it would be nice to fix them before merging.
Otherwise I'm happy to !

Anything else to add @iamthomasbishop about the listed fixes?

extensions/blocks/contact-info/style.native.scss

…orPlainTextColor. Removing extraneous return statement.

…n.native.js.

… font.

illusaen · 2020-09-28T13:59:12Z

In discussion with @etoledom and @hypest, will split the font weight issue from this main contact info PR in order to keep the PR from hanging longer. @iamthomasbishop, does everything else look alright? Please also take a look at PR #2616 for corresponding Appium UI Tests.

iamthomasbishop · 2020-10-03T18:04:01Z

@illusaen As far as I can tell from the screenshots above, everything looks pretty solid from a visual standpoint. Provided @etoledom has reviewed the tech side, I think we can move forward and make visual refinements if/where necessary. Nice work!

maxme

Good job @illusaen - feature looks good to me!

@iamthomasbishop here is a test apk if you want to test it.

etoledom

Approving based on the tests performed here

Great job @illusaen !

extensions/blocks/contact-info/editor.native.scss

extensions/blocks/contact-info/edit.native.js

extensions/blocks/contact-info/style.native.scss

Co-authored-by: Jeremy Herve <jeremy@tagada.hu>

extensions/blocks/contact-info/edit.native.js

extensions/blocks/contact-info/editor.native.scss

extensions/blocks/contact-info/style.native.scss

etoledom · 2020-10-13T16:26:53Z

Thank you @jeherve for your review!
I have updated the branch with the requested changes.

etoledom

Tested after the latest changes and everything seems to work as expected 👍

jeherve · 2020-10-14T15:17:40Z

r215211-wpcom

illusaen added 4 commits September 8, 2020 00:43

WC: Adding specialized keyboard to email/phone number forms in contac…

21f8012

…t-info. Moving google map setting to address block setting.

WC: Adding styles to contact-info block placeholder. Simplifying addr…

137a1ff

…ess fragments to use map instead of listing each entry.

WC: Not selecting inner block when inserting a contact info block.

572bbf6

WC: Updating contact info block settings to have title. Adding getSty…

144369a

…lesFromColorScheme to placeholderTextColor for dark mode. Using Rich Text placeholder text color.

matticbot added the Touches WP.com Files label Sep 10, 2020

jeherve added [Block] Contact Info [Status] In Progress [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Sep 10, 2020

jeherve assigned illusaen Sep 10, 2020

jeherve changed the title ~~Contact info~~ Contact Info Block: changes for adaptation into a mobile block Sep 10, 2020

etoledom self-requested a review September 10, 2020 12:10

etoledom reviewed Sep 11, 2020

View reviewed changes

extensions/blocks/contact-info/address/edit.native.js Outdated Show resolved Hide resolved

extensions/blocks/contact-info/phone/edit.native.js Outdated Show resolved Hide resolved

illusaen added 2 commits September 11, 2020 23:04

WC: Resetting jetpack web to original state. Refactored styling furth…

856dae5

…er on contact-info/address/edit in order to remove repeated code.

WC: Changing Contact Info Settings to Address Settings. Adding color …

0ad9730

…for text and text-decoration for light and dark modes. Removing unused code.

illusaen mentioned this pull request Sep 14, 2020

Update jetpack wordpress-mobile/gutenberg-mobile#2616

Closed

etoledom reviewed Sep 15, 2020

View reviewed changes

extensions/blocks/contact-info/style.native.scss Outdated Show resolved Hide resolved

illusaen added 3 commits September 15, 2020 04:24

WC: Refactoring styles to combine blockEditorPlainText with blockEdit…

2ff4940

…orPlainTextColor. Removing extraneous return statement.

WC: Refactoring common elements in PhoneEdit and EmailEdit into commo…

8315acf

…n.native.js.

WC: Adding unique key to FlatList. Making shadow color transparent on…

bf9116e

… font.

etoledom self-requested a review September 30, 2020 10:30

etoledom added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Oct 5, 2020

jeherve added this to the 9.1 milestone Oct 5, 2020

jeherve added the [Status] Needs Team Review label Oct 9, 2020

maxme self-requested a review October 9, 2020 10:30

maxme previously approved these changes Oct 9, 2020

View reviewed changes

etoledom previously approved these changes Oct 9, 2020

View reviewed changes

jeherve removed the [Status] Needs Team Review label Oct 9, 2020

jeherve requested changes Oct 9, 2020

View reviewed changes

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 9, 2020

Update extensions/blocks/contact-info/style.native.scss

65a7f2e

Co-authored-by: Jeremy Herve <jeremy@tagada.hu>

etoledom dismissed stale reviews from maxme and themself via 65a7f2e October 13, 2020 16:02

etoledom reviewed Oct 13, 2020

View reviewed changes

extensions/blocks/contact-info/edit.native.js Outdated Show resolved Hide resolved

Replace indentation with tabs

6b19ce9

etoledom reviewed Oct 13, 2020

View reviewed changes

extensions/blocks/contact-info/editor.native.scss Outdated Show resolved Hide resolved

Removing import not needed

2c362d9

etoledom reviewed Oct 13, 2020

View reviewed changes

extensions/blocks/contact-info/style.native.scss Outdated Show resolved Hide resolved

Replace indentation with Tabs

39c0de6

etoledom approved these changes Oct 13, 2020

View reviewed changes

jeherve approved these changes Oct 14, 2020

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Oct 14, 2020

jeherve merged commit ff65621 into Automattic:master Oct 14, 2020

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 14, 2020

jeherve removed the [Status] Needs Changelog label Oct 14, 2020

etoledom mentioned this pull request Oct 15, 2020

Update Jetpack ref wordpress-mobile/gutenberg-mobile#2729

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contact Info Block: changes for adaptation into a mobile block #17123

Contact Info Block: changes for adaptation into a mobile block #17123

illusaen commented Sep 10, 2020 •

edited

Loading

jetpackbot commented Sep 10, 2020 •

edited

Loading

etoledom left a comment

etoledom left a comment

illusaen commented Sep 28, 2020

iamthomasbishop commented Oct 3, 2020

maxme left a comment

etoledom left a comment

etoledom commented Oct 13, 2020

etoledom left a comment

jeherve commented Oct 14, 2020

Contact Info Block: changes for adaptation into a mobile block #17123

Contact Info Block: changes for adaptation into a mobile block #17123

Conversation

illusaen commented Sep 10, 2020 • edited Loading

WIP Design

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Proposed changelog entry for your changes:

Screenshots

jetpackbot commented Sep 10, 2020 • edited Loading

etoledom left a comment

Choose a reason for hiding this comment

etoledom left a comment

Choose a reason for hiding this comment

illusaen commented Sep 28, 2020

iamthomasbishop commented Oct 3, 2020

maxme left a comment

Choose a reason for hiding this comment

etoledom left a comment

Choose a reason for hiding this comment

etoledom commented Oct 13, 2020

etoledom left a comment

Choose a reason for hiding this comment

jeherve commented Oct 14, 2020

illusaen commented Sep 10, 2020 •

edited

Loading

jetpackbot commented Sep 10, 2020 •

edited

Loading