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] elevate scrollEnabled prop to BlockList #21505

Merged
merged 2 commits into from
Apr 16, 2020

Conversation

jbinda
Copy link
Contributor

@jbinda jbinda commented Apr 9, 2020

Description

This PR moves calculation of scrollEnabled prop to BLockList instead of passing it from blocks.

Please also refer to:
Related gutenberg-mobile PR
Related gutenberg-mobile issue

How has this been tested?

  1. You can start with below initial-html
/**
 * @format
 * @flow
 */

export default `
<!-- wp:group -->
<div class="wp-block-group"><div class="wp-block-group__inner-container"><!-- wp:heading -->
<h2>Test Heading</h2>
<!-- /wp:heading --></div></div>
<!-- /wp:group -->

<!-- wp:group -->
<div class="wp-block-group"><div class="wp-block-group__inner-container"></div></div>
<!-- /wp:group -->

<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:heading -->
<h2>Test Heading</h2>
<!-- /wp:heading -->

<!-- wp:image -->
<figure class="wp-block-image"><img alt=""/></figure>
<!-- /wp:image --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:image -->
<figure class="wp-block-image"><img alt=""/></figure>
<!-- /wp:image --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
`;
  1. Feel free to add more nested structure
  2. Focus mainly on nested Columns and Group block
  3. Try to scroll nested blocks (it should not be possible - see screenshot with unexpected behaviour below)

Screenshots

This PR prevents from below scenario. I have caught this during work on Columns block and I didnt see any other place that it can happens. However it might happens in the future in other block as well (most probably for block in horizontal layout)

scroll bug

Types of changes

Refactor

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.

@jbinda jbinda added [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P 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 9, 2020
@jbinda jbinda requested review from Tug, geriux, pinarol and dratwas April 9, 2020 14:08
@jbinda jbinda self-assigned this Apr 9, 2020
@@ -140,7 +139,7 @@ export class BlockList extends Component {
! isRootList && styles.overflowVisible,
] }
horizontal={ horizontal }
scrollEnabled={ scrollEnabled }
scrollEnabled={ isRootList }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one should lock scrolling of nested InnerBlocks. However I would like to make sure if it do not block something that we don't want to block.

The reason to introduce that prop lays in Column block PR . In nested structure there was possibility to drag content of Column or Group block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you comment this line completely you will notice the behaviour presented on GIF in the description

@github-actions
Copy link

github-actions bot commented Apr 9, 2020

Size Change: 0 B

Total Size: 839 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.01 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 104 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.11 kB 0 B
build/block-library/editor.css 7.11 kB 0 B
build/block-library/index.js 112 kB 0 B
build/block-library/style-rtl.css 7.13 kB 0 B
build/block-library/style.css 7.14 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.7 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.6 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.1 kB 0 B
build/data-controls/index.js 1.25 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 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.1 kB 0 B
build/edit-navigation/index.js 3.1 kB 0 B
build/edit-navigation/style-rtl.css 279 B 0 B
build/edit-navigation/style.css 280 B 0 B
build/edit-post/index.js 27.8 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.4 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.53 kB 0 B
build/edit-widgets/style-rtl.css 4.65 kB 0 B
build/edit-widgets/style.css 4.64 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.6 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.29 kB 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 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.91 kB 0 B
build/list-reusable-blocks/index.js 3.12 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 5.28 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 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.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 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.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@dratwas dratwas left a comment

Choose a reason for hiding this comment

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

Checked and everything works fine

@jbinda jbinda merged commit 13f244b into master Apr 16, 2020
@jbinda jbinda deleted the rnmobile/refactor/elevate-scrollEnabled branch April 16, 2020 05:34
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P 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.

2 participants