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] Adjust new borders implementations to columns PR #21073

Merged
merged 33 commits into from
Mar 26, 2020

Conversation

dratwas
Copy link
Contributor

@dratwas dratwas commented Mar 23, 2020

Description

This PR brings adjustments need to allow using new bordering logic inside Column Block.

Please refer to:
Original refactoring PR
Column Block PR
design note

Builds for testing:
iOS: IPA 25710
Andorid: APK 42426 (please note one issue commented here)

How has this been tested?

Checking all Column Block PR testing requirements (see Column PR description)

During those checks please keep attention to how border is changing and see if there is no "jumping" during selection change

Types of changes

Refactor - Change the way how blocks bordering style is applied.

List of changes:

  • bring new way how to style bordering is applied for blocks
  • adjustments needed for Columns/Column block

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.

@github-actions
Copy link

github-actions bot commented Mar 23, 2020

Size Change: 0 B

Total Size: 860 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 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 102 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.23 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.44 kB 0 B
build/block-library/style.css 7.45 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/blocks/index.js 57.5 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.25 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 771 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.2 kB 0 B
build/edit-post/style-rtl.css 8.47 kB 0 B
build/edit-post/style.css 8.46 kB 0 B
build/edit-site/index.js 6.72 kB 0 B
build/edit-site/style-rtl.css 2.88 kB 0 B
build/edit-site/style.css 2.88 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 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.8 kB 0 B
build/editor/style-rtl.css 4 kB 0 B
build/editor/style.css 3.98 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 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 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.49 kB 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.69 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/notices/index.js 1.57 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/priority-queue/index.js 781 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

Copy link
Contributor

@jbinda jbinda left a comment

Choose a reason for hiding this comment

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

I have checked the code and test on demo app. It is huge improvement about usage and brings a lot of benefits - specially in long term.

I have also left some comments with minor adjustments (discussed on a call already). Hope we will be able to merge those changes soon ! :)

packages/base-styles/_variables.scss Show resolved Hide resolved
Comment on lines +43 to +45
if ( columnsInRow > 1 ) {
const margins = columnsInRow * 2 * styles.columnMargin.marginLeft;
width = ( minWidth - margins ) / Math.max( 1, columnsInRow );
Copy link
Contributor

Choose a reason for hiding this comment

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

I have checked and this also works for more than 2 columnsInRow.

However we need to implement ideal version of FloatingToolbar because it breaks the layout when there is no enough space (it has minWIdth and the Column container is stretch to this sizes - see below screen). It's not something we need to change here just as reminder)

Unselected Columns layout

Selected Column with broken Columns layout

Selected Column with proper Columns layout after disable FloatingToolbar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's known issue and exists also in buttons block

@jbinda jbinda mentioned this pull request Mar 25, 2020
6 tasks
@jbinda jbinda marked this pull request as ready for review March 26, 2020 15:39
@jbinda jbinda self-assigned this Mar 26, 2020
@jbinda jbinda added [Block] Columns Affects the Columns Block Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Mar 26, 2020
@jbinda jbinda changed the title Adjust new borders implementations to columns PR [RNMobile] Adjust new borders implementations to columns PR Mar 26, 2020
@jbinda jbinda merged commit 37bad85 into rnmobile/column-block Mar 26, 2020
@jbinda jbinda deleted the rnmobile/column-block-borders branch March 26, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns 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.

2 participants