Skip to content

[RNMobile] Button Block#18823

Merged
lukewalczak merged 55 commits intomasterfrom
rnmobile/button-block
Feb 6, 2020
Merged

[RNMobile] Button Block#18823
lukewalczak merged 55 commits intomasterfrom
rnmobile/button-block

Conversation

@lukewalczak
Copy link
Copy Markdown
Member

@lukewalczak lukewalczak commented Nov 29, 2019 •
edited
Loading

Fixes wordpress-mobile/gutenberg-mobile#1513

Ref to gb-mobile: wordpress-mobile/gutenberg-mobile#1616
Ref to WordPress-Android: wordpress-mobile/WordPress-Android#10902
Ref to WordPress-iOS:wordpress-mobile/WordPress-iOS#13060

This PR introduces 1st iteration of mobile Button block component.

To achieve gradient background color on mobile react-native-linear-gradient comes in handy.

The first iteration includes:

  • Rendering Button with:
    • gradient background color ✅
    • custom background color chosen from picker ✅
    • custom text color chosen from picker ✅
  • Options to change border radius, add link, open url in new tab ✅
  • Expanding and shrinking RichText ✅
  • Notification for missing color settings functionality ✅

Blockers:

  • cannot render button with background color chosen from color palette ❌
  • cannot render button with text color chosen from color palette ❌
    * issue with expanding and shrinking RichText on iOS ❌
    * issue with shrinking RichText on Android ❌
    * issue with text color on WordPress-iOS ❌

To test:

-> Gradient

  • open draft post on web
  • add Button
  • choose the gradient background color
  • save post
  • open draft post on mobile
  • check if a gradient is correct

-> Custom background color

  • open draft post on web
  • add Button
  • choose the custom background color from picker
  • save post
  • open draft post on mobile
  • check if a background color is correct

-> Custom text color

  • open draft post on web
  • add Button
  • choose the custom text color from picker
  • save post
  • open draft post on mobile
  • check if a text color is correct

-> Border radius

  • open draft post on mobile
  • add Button
  • open settings
  • move slider to change border radius value

-> Add link

  • open draft post on mobile
  • add Button
  • open settings
  • pass a url
  • switch on open in new tab
  • open a preview mode
  • click Button and check if it opens a new window with passed url

Screenshot / gifs:

ANDROID

regular linear gradient
Screenshot 2019-12-02 at 10 35 50 Screenshot 2019-12-02 at 10 45 54

IOS

regular linear gradient
Screenshot 2019-12-02 at 11 05 07 Screenshot 2019-12-02 at 10 58 11

IOS Dark Mode

regular linear gradient
Screenshot 2019-12-02 at 11 12 28 Screenshot 2019-12-02 at 11 12 43

Expanding and shrinking RichText

android ios
android_rich_text ios_rich_text

Settings

ios android
Screenshot 2019-12-02 at 11 33 20 Screenshot 2019-12-02 at 11 05 49

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

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. .

@lukewalczak lukewalczak added [Block] Buttons Affects the Buttons Block [Status] In Progress Tracking issues with work in progress labels Nov 29, 2019
@lukewalczak lukewalczak self-assigned this Nov 29, 2019
@lukewalczak lukewalczak added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Nov 29, 2019
@lukewalczak lukewalczak force-pushed the rnmobile/button-block branch from 439466b to 90a60e0 Compare December 2, 2019 10:22
@lukewalczak lukewalczak force-pushed the rnmobile/button-block branch from 90a60e0 to 1fc7530 Compare December 2, 2019 10:29
@lukewalczak lukewalczak mentioned this pull request Dec 2, 2019
Merged
1 task
@lukewalczak
Copy link
Copy Markdown
Member Author

lukewalczak commented Dec 4, 2019 •
edited
Loading

UPDATES

  • new fallback color
  • notification bottom sheet
regular settings notification
Screenshot 2019-12-04 at 12 45 49 Screenshot 2019-12-04 at 12 07 07 Screenshot 2019-12-04 at 12 07 17
regular settings notification
Screenshot 2019-12-04 at 12 47 52 Screenshot 2019-12-04 at 11 18 42 Screenshot 2019-12-04 at 11 18 45
regular settings notification
Screenshot 2019-12-04 at 12 48 42 Screenshot 2019-12-04 at 12 48 46 Screenshot 2019-12-04 at 12 48 51
ios android
ios_notif android_notif

@lukewalczak lukewalczak force-pushed the rnmobile/button-block branch from 32a9934 to 148ff59 Compare December 6, 2019 14:21
@lukewalczak lukewalczak marked this pull request as ready for review December 9, 2019 08:41
@lukewalczak lukewalczak removed the [Status] In Progress Tracking issues with work in progress label Dec 9, 2019
This was referenced Dec 9, 2019
Merged
Merged
@iamthomasbishop
Copy link
Copy Markdown

iamthomasbishop commented Jan 21, 2020

@lukewalczak Regarding the link rel icon part, it's looking good in the last screenshot above. 👍

@lukewalczak
Copy link
Copy Markdown
Member Author

lukewalczak commented Jan 21, 2020 •
edited
Loading

@pinarol Thanks for reporting that weird behaviour, which I didn't notice before.
I'm not sure if it's related specifically with that PR, because I'm also able to reproduce it on develop, please check pasted gif.

I saved in clipboard www.wordpress.com/read.
Then I'm opening a new post draft, and adding fresh Heading component. Next clicking the link icon and actually link is already in URL field.

Can you please check if you're able to follow my steps and reproduce it as well?

@pinarol
Copy link
Copy Markdown
Contributor

pinarol commented Jan 21, 2020

Then I'm opening a new post draft, and adding fresh Heading component. Next clicking the link icon and actually link is already in URL field.

Yes but it shouldn't override the already existing link. When I try on Android with an already existing Button, it is overriding that Button's link as well.

@pinarol
Copy link
Copy Markdown
Contributor

pinarol commented Jan 21, 2020

I just tested with WPiOS as well, and couldn't generate any odd behavior, here are the things I've tested:

  • Custom colors are rendered with no problem, both for the text and background

IMG_3362

  • Theme colors fallback to this gray background and white font:

IMG_3358

  • New Buttons are created with the same color in web:

IMG_3359

  • Gradient colors are working well:

IMG_3360

  • Border radius is working
  • Button max width is calculated OK inside inner blocks
  • Bottom sheets ok with light&dark modes

Links:

  • Created a link url on web and verified it is matching the value on mobile
  • Created a link on mobile and see it is working it in the page preview
  • Checked if 'Open in new tab' option works
  • Created a link rel on web and verified it is matching the value on mobile and vice versa
  • Remove link button clears everything in the link bottom sheet

So after we sort out the link override on Android, this should be ready to go. I haven't checked the latest version of the code yet, I'll do that tomorrow.

@lukewalczak
Copy link
Copy Markdown
Member Author

lukewalczak commented Jan 21, 2020

Yes but it shouldn't override the already existing link. When I try on Android with an already existing Button, it is overriding that Button's link as well.

I see, will investigate it!

pinarol
pinarol reviewed Jan 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

Dropping some code review comments.

pinarol
pinarol reviewed Jan 31, 2020
View reviewed changes
@pinarol
Copy link
Copy Markdown
Contributor

pinarol commented Jan 31, 2020 •
edited
Loading

I am happy we reverted changes to link/modal.native.js, this way it looks and works much better. IMO, after addressing the duplications this is ready to go. I tested this on WPAndroid and working ok. I'll test one more time after the possible code changes.

@lukewalczak lukewalczak force-pushed the rnmobile/button-block branch from 9109463 to e6452d4 Compare February 4, 2020 13:46
@lukewalczak
Copy link
Copy Markdown
Member Author

lukewalczak commented Feb 4, 2020

UPDATES

  1. Reused code for controls within bottom sheets
  2. Add support for selection color

pinarol
pinarol approved these changes Feb 5, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉 Great job!
I repeated my tests on both platforms.

@pinarol pinarol mentioned this pull request Feb 6, 2020
Closed
@lukewalczak lukewalczak merged commit 8313c65 into master Feb 6, 2020
@lukewalczak lukewalczak deleted the rnmobile/button-block branch February 6, 2020 15:03
@github-actions github-actions bot added this to the Gutenberg 7.5 milestone Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@koke koke koke left review comments

@iamthomasbishop iamthomasbishop iamthomasbishop left review comments

@dratwas dratwas dratwas left review comments

@pinarol pinarol pinarol approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@Soean Soean Awaiting requested review from Soean

@talldan talldan Awaiting requested review from talldan

Assignees

@lukewalczak lukewalczak

Labels

[Block] Buttons Affects the Buttons Block 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

Milestone

Gutenberg 7.5

Development

Successfully merging this pull request may close these issues.

Button block - 1st iteration

5 participants

@lukewalczak @pinarol @iamthomasbishop @koke @dratwas