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] Replace Slider #18806

Merged
merged 5 commits into from
Dec 5, 2019
Merged

[RNMobile] Replace Slider #18806

merged 5 commits into from
Dec 5, 2019

Conversation

lukewalczak
Copy link
Member

@lukewalczak lukewalczak commented Nov 28, 2019

Description

I added within gutenberg-mobile new library react-native-slider which is going to replace Slider from react-native which will be deprecated soon and what's more have a bug on ios ~13.

Ref to gb-mobile:wordpress-mobile/gutenberg-mobile#1620
Ref to WordPress-Android:wordpress-mobile/WordPress-Android#10871
Ref to WordPress-iOS:wordpress-mobile/WordPress-iOS#13019

How has this been tested?

I was testing it in WordPress-Android, WordPress-iOS and also within a playground.

  1. Add Spacer block on mobile
  2. Play with Slider
  • I suggest you also to set small range eg. minValue=0, maxValue=20 in RangeCell to check if Slider is not jumping

Screenshots

Types of changes

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 Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Block] Gallery Affects the Gallery Block - used to display groups of images [Block] Spacer Affects the Spacer Block labels Nov 28, 2019
@lukewalczak lukewalczak self-assigned this Nov 28, 2019
@pinarol
Copy link
Contributor

pinarol commented Nov 28, 2019

I suggest you also to set small range eg. minValue=0, maxValue=20 in RangeCell to check if Slider is not jumping

Actually this is a pretty important test case since that jumpiness on iOS 13.x is the main reason we wanted to make this change.

@pinarol pinarol requested review from etoledom and marecar3 November 28, 2019 15:59
@pinarol
Copy link
Contributor

pinarol commented Nov 29, 2019

Could you also add the gif that demonstrates the odd behavior @lukewalczak ? It is not clear for reviewers what we are fixing.

@lukewalczak
Copy link
Member Author

Sure! Weird behavior:
odd

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@marecar3 marecar3 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 Android and it's working good!
LGTM!

@lukewalczak lukewalczak merged commit bb84493 into master Dec 5, 2019
@lukewalczak lukewalczak deleted the rnmobile/feat/update-slider branch December 5, 2019 08:50
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Block] Spacer Affects the Spacer 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
Development

Successfully merging this pull request may close these issues.

5 participants