[RNMobile] FloatingToolbar #17399

lukewalczak · 2019-09-10T15:57:39Z

Description

PR is adding FloatingToolbar component without inner functionality.
gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#1362

How has this been tested?

Tested on both platforms

Screenshots

ios	android

Types of changes

Added FloatingToolbar.

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.

* [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (WordPress#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (WordPress#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (WordPress#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (WordPress#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (WordPress#17193)

* First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots

…#17268)

…17260)

Previously, tapping at the end of the post would insert a block immediately after the currently selected block. In addition, this commit is cleaning out a few unusued props in the block-list file.

* First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots * Support group block on mobile * Extend shouldShowInsertionPoint condition to be false when group is selected * Code refactor * Update package-lock

* Remove the need to import `useStyle` and pass the theme prop on every instance that `withStyle` is used * Implement dark-mode refactor on all components * Fix broken native tests * Fix default block appender background color on DarkMode * DarkMode: Make `useStyle` a class function

packages/block-editor/src/components/block-list/block.native.js

* [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (WordPress#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (WordPress#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (WordPress#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (WordPress#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (WordPress#17193) * Add a preliminary version of the AutosaveMonitor for mobile that calls the "bridge" and asks the native side to save the content * Add autosave mock function for tests * Fix merge conflicts * Fix lint * Re-add autosave on mobile that was removed erroneously during import-merge from rnmobile/master * Remove native variant of AutosaveMonitor and introduces changes at editor store level * Default to false for `isEditedPostAutosaveable` on mobile. There was a typo in the returing value on the previous commit. * Make sure to consider edits to the Title when checking if auto-save is needed * Fix lint

koke · 2019-09-30T15:31:58Z

I was trying to understand the issues better, and while I don't have any good suggestions yet, I noticed this while using the UI inspector in Xcode, which doesn't look right:

lukewalczak · 2019-09-30T15:47:26Z

@koke Can it be caused by a high zIndex value (100) ?

koke · 2019-10-01T09:36:27Z

@koke Can it be caused by a high zIndex value (100) ?

No, it seems FloatingToolbar is being rendered for every block:

lukewalczak · 2019-10-01T09:51:21Z

@koke Thanks! The problem was that FloatingToolbar was rendered actually for every block, but now I've made it conditional to flag showDisplayToolbar.

pinarol

Thanks for the effort @lukewalczak Just dropping a few comments, aside from that I think it looks good.

About the re-occuring FloatingToobar problem @koke reported, could you open an issue? I don't think we should solve it with this PR but we should track it, there might be something wrong with slot-fill. Considering that we might want to change the FloatingToolbar designs let's not block this PR with that issue.

pinarol · 2019-10-01T12:20:30Z

packages/block-editor/src/components/block-list/index.native.js

@@ -93,6 +96,9 @@ export class BlockList extends Component {
 					ListHeaderComponent={ header }
 					ListEmptyComponent={ this.renderDefaultBlockAppender }
 					ListFooterComponent={ withFooter && this.renderBlockListFooter }
+					getItemLayout={ ( data, index ) => {


Do we still need this considering the current temporary fix? If we don't let's remove it to avoid having unexpected side effects

Yes, it's still needed to get working properly FloatingToolbar on Android.

pinarol · 2019-10-01T12:24:08Z

packages/block-editor/src/components/block-list/index.native.js

 		return (
 			<View
 				style={ { flex: 1 } }
 				onAccessibilityEscape={ clearSelectedBlock }
 			>
+				{ showFloatingToolbar && <FloatingToolbar.Slot fillProps={ { innerFloatingToolbar: showFloatingToolbar } } /> }


Do we still need to change this file? considering we are rendering the first one inside the bounds. I know this is a temp fix but still I'd want to change minimum code to avoid any side effects until we find a better solution.

If we want to simplify it to the minimum we don't need to use slot/fill, however, I've decided to keep this for the next investigations.

mkevins · 2019-10-03T04:00:59Z

The temporary workaround seems to keep the floating toolbar within the bounds of the parent, so taps are not "falling through" anymore. But, the toolbar currently prevents visibility of the underlying content, which is problematic:

Is this expected behavior for this temporary iteration, or are we expecting the content to clear the toolbar?

cc: @pinarol

pinarol · 2019-10-03T07:15:02Z

Is this expected behavior for this temporary iteration

Yes, this won’t go to prod as is, we’ll have another iteration. For now we are trying to unblock dependent PRs. @mkevins

pinarol · 2019-10-03T08:37:08Z

@lukewalczak Could you open an issue in the gutenberg-mobile repo with the screenshot here just for us to not forget about it: #17399 (comment) ? After that it seems ready to merge to me.

mkevins

Approving for first iteration, so other dependent PRs can move forward. 👍

pinarol · 2019-10-03T11:51:04Z

@lukewalczak Could you open an issue in the gutenberg-mobile repo with the screenshot here just for us to not forget about it: #17399 (comment) ? After that it seems ready to merge to me.

Taking this back because I tried with the latest version and it is all fixed

pinarol

LGTM 🎉 We'll do separate iterations to make this better, for now let's merge for not blocking other PRs

etoledom and others added 13 commits August 28, 2019 10:31

Activate Travis CI on rnmobile/master branch (WordPress#17229)

643c1b2

MediaUpload and MediaPlaceholder unify props (WordPress#17145)

3db95b7

Unify media placeholder and upload props within media-text (WordPress…

7aa44a2

…#17268)

[RNMobile] Fix dismiss keyboard button for the post title (WordPress#…

f9fa455

…17260)

Recover border colors (WordPress#17269)

7b12673

[RNMobile] Insure tapping at end of post inserts at end

14d482b

Previously, tapping at the end of the post would insert a block immediately after the currently selected block. In addition, this commit is cleaning out a few unusued props in the block-list file.

Remove redundant bg color within button appender (WordPress#17325)

fc8c3da

Add FloatingToolbar component

654e162

Add visible overflow to flat list wrapper

111059c

lukewalczak requested review from ellatrix, talldan and youknowriad as code owners September 10, 2019 15:57

Remove useless codelines

89ed4d7

lukewalczak mentioned this pull request Sep 10, 2019

Add FloatingToolbar component wordpress-mobile/gutenberg-mobile#1362

Merged

1 task

lukewalczak added 2 commits September 10, 2019 18:47

Update test

900233d

Trigger CI

2deb517

pinarol reviewed Sep 10, 2019

View reviewed changes

packages/block-editor/src/components/block-list/block.native.js Outdated Show resolved Hide resolved

pinarol reviewed Sep 10, 2019

View reviewed changes

packages/block-editor/src/components/block-list/block.native.js Outdated Show resolved Hide resolved

lukewalczak added 2 commits September 10, 2019 21:28

Remove redundant conditions

6106f27

Trigger CI

67c5cca

dratwas mentioned this pull request Sep 18, 2019

[RNMobile] floating toolbar disappear mechanism #17466

Closed

5 tasks

jbinda mentioned this pull request Sep 20, 2019

[RNMobile] Inner Block Navigate upward through hierarchy #17496

Merged

5 tasks

dratwas mentioned this pull request Sep 20, 2019

[rnmobile] Breadcrumbs #17471

Merged

5 tasks

Trigger CI

9659a73

jbinda mentioned this pull request Sep 24, 2019

[RNMobile] Inner Block Navigate down through hierarchy #17547

Merged

5 tasks

Small code refactor

47cd600

lukewalczak changed the base branch from rnmobile/master to master October 1, 2019 06:43

lukewalczak changed the base branch from master to rnmobile/master October 1, 2019 06:51

lukewalczak added 2 commits October 1, 2019 10:05

Use temporary solution for first item in group

6449038

Merge branch 'rnmobile/master' into rnmobile/floating-toolbar

d0ba0fe

lukewalczak changed the base branch from rnmobile/master to master October 1, 2019 08:11

lukewalczak added 3 commits October 1, 2019 10:16

Merge branch 'master' into rnmobile/floating-toolbar

c70edc5

Merge fix

300a8a5

Merge fix

a9d7489

Make FloatingToolbar conditional to show flag

2695248

lukewalczak added 2 commits October 1, 2019 14:06

Fix after merging

d2a07ec

Add check for Media&Text block

e911f5d

pinarol reviewed Oct 1, 2019

View reviewed changes

mkevins approved these changes Oct 3, 2019

View reviewed changes

pinarol approved these changes Oct 3, 2019

View reviewed changes

lukewalczak merged commit 0e0b65f into WordPress:master Oct 3, 2019

lukewalczak deleted the rnmobile/floating-toolbar branch October 3, 2019 13:52

This was referenced Oct 4, 2019

[WIP][RNMobile][FIX] floating toolbar for the first element in a group #17769

Closed

[FIX] Floating toolbar wordpress-mobile/gutenberg-mobile#1409

Closed

etoledom mentioned this pull request Oct 9, 2019

[iOS] SafeArea is drawn behind block content wordpress-mobile/gutenberg-mobile#1424

Closed

youknowriad added this to the Gutenberg 6.7 milestone Oct 14, 2019

pinarol mentioned this pull request Feb 12, 2020

FloatingToolbar ideal version wordpress-mobile/gutenberg-mobile#1414

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RNMobile] FloatingToolbar #17399

[RNMobile] FloatingToolbar #17399

lukewalczak commented Sep 10, 2019 •

edited

Loading

koke commented Sep 30, 2019

lukewalczak commented Sep 30, 2019

koke commented Oct 1, 2019

lukewalczak commented Oct 1, 2019

pinarol left a comment

pinarol Oct 1, 2019

lukewalczak Oct 1, 2019

pinarol Oct 1, 2019

lukewalczak Oct 1, 2019 •

edited

Loading

mkevins commented Oct 3, 2019

pinarol commented Oct 3, 2019 •

edited

Loading

pinarol commented Oct 3, 2019 •

edited

Loading

mkevins left a comment

pinarol commented Oct 3, 2019

pinarol left a comment

[RNMobile] FloatingToolbar #17399

[RNMobile] FloatingToolbar #17399

Conversation

lukewalczak commented Sep 10, 2019 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

koke commented Sep 30, 2019

lukewalczak commented Sep 30, 2019

koke commented Oct 1, 2019

lukewalczak commented Oct 1, 2019

pinarol left a comment

Choose a reason for hiding this comment

pinarol Oct 1, 2019

Choose a reason for hiding this comment

lukewalczak Oct 1, 2019

Choose a reason for hiding this comment

pinarol Oct 1, 2019

Choose a reason for hiding this comment

lukewalczak Oct 1, 2019 • edited Loading

Choose a reason for hiding this comment

mkevins commented Oct 3, 2019

pinarol commented Oct 3, 2019 • edited Loading

pinarol commented Oct 3, 2019 • edited Loading

mkevins left a comment

Choose a reason for hiding this comment

pinarol commented Oct 3, 2019

pinarol left a comment

Choose a reason for hiding this comment

lukewalczak commented Sep 10, 2019 •

edited

Loading

lukewalczak Oct 1, 2019 •

edited

Loading

pinarol commented Oct 3, 2019 •

edited

Loading

pinarol commented Oct 3, 2019 •

edited

Loading