[Mobile] Create cover block with custom colour (without an image) #23870

antonis · 2020-07-10T12:23:36Z

Fixes #23868 (parent wordpress-mobile/gutenberg-mobile#2275)

Related PRs:

Gutenberg-Mobile Create cover block with custom colour (without an image) wordpress-mobile/gutenberg-mobile#2508
Depends on Create cover block with only colour from a default palette [Mobile] Create cover block with only colour (from a default palette) without an image #23833
Clear media button in block settings [Mobile] Cover: Adds Clear media button in block settings #23794
Media Button when only colour is selected [Mobile] Cover: Add Media Button when only colour is selected #23878

Description

Additionally to the palette with four default colours a Custom Colour Swatch has been added. When the user taps it, the custom colour picker UI shows. The user can tap "Cancel" to go back, or "Apply" to proceed with applying the colour.

How has this been tested?

Cover block creation with custom colour

Add a cover block
Press the one custom colour from the palette
Verify that the custom colour picker appears
Select a colour and press apply
Verify that a new cover block with only colour is created

Canceling the cover block creation

Add a cover block
Press the one custom colour from the palette
Verify that the custom colour picker appears
Select a colour and press cancel
Verify that the block remains in the original state

Screenshots

Empty cover block	Cover block with only colour

Types of changes

Enhancement

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.

antonis · 2020-07-10T12:28:45Z

Hello @iamthomasbishop 👋
You can check an implementation of the cover flow creation with custom colour at the screenshots above.
The creation of a cover block by selecting from a set of default colours is implemented with #23833.
Wdyt?

iamthomasbishop · 2020-07-10T22:34:59Z

@antonis Excited to see this in action! I have a few minor requests. I don't know how accurate my Figma file is in terms of margins and padding at the top/bottom of the background cell on the placeholder, but here's what I have in mind:

Can we move the swatches up a line so that the action button ("add image or video") sits below them?
Swatches can be ~32px, with 10px gutters

Here's a visual of what I'm imagining:

Mockup	Metrics

The transition is a bit jarring/abrupt when you apply a color. Is there any way we can apply a smooth transition when a color is chosen so it's not so jarring? (And bonus points if we can also do the next bullet point in tandem)
Another thing that might help smooth the rough edges is to apply the default height to the placeholder as well, so when the background is set, it's not a large jump from ~140px to 300px (our default height). That would look like this:

This is looking pretty solid already. Nice work!

…th_custom_color

antonis · 2020-07-14T11:08:09Z

Hello @iamthomasbishop 👋
Thank you for the detailed feedback on this. I've worked on this and I'd like to share some draft screens (not pixel perfect yet dimension wise).

Default color selection	Custom color selection	Other blocks

To create the above I needed to customise the Media Placeholder component also used in other blocks (e.g. Image, MediaText). Related to this I have the following questions:

Should I use the new metrics (e.g. the distance-4 between the cover icon and the cover label) in all blocks using the Media Placeholder?
Should I use the increased placeholder height (~140px to 300px) in all places or just the cover block?

…th_custom_color

…th_custom_color # Conflicts: # packages/block-library/src/cover/style.native.scss # packages/components/src/color-palette/index.native.js

iamthomasbishop · 2020-07-21T16:13:36Z

@antonis The dynamic background color looks great, but I think it causes some friction with the content that is over it. I think we should keep the dynamic adjustment, so can we try either dimming (to something like ~30-50%, perhaps?) or hiding the contents of the placeholder (block icon, title, swatches, action) -- temporarily, while the user is selecting a custom color. Then when they cancel or apply, the contents would re-appear fully.

Note: this should only apply to the starter state of the Cover block. I can't think of any other blocks off the top of my head that this would affect.

iamthomasbishop · 2020-07-21T16:16:11Z

@antonis One more tiny thing -- Can we use the same text style as the footer label on the "Text Color" setting (on a Button, for example):

antonis · 2020-07-22T16:27:30Z

Hello @iamthomasbishop 👋 ,

I updated the PR with the suggested changes. Please let me know wdyt :)

Light mode	Dark mode

iamthomasbishop · 2020-07-22T16:46:07Z

Looks great! I think we can ship this!

chipsnyder · 2020-07-22T17:02:17Z

packages/block-editor/src/components/inspector-controls/index.native.js

@@ -30,7 +39,9 @@ const FillWithSettingsButton = ( { children, ...props } ) => {
 					</BottomSheetConsumer>
 				}
 			</Fill>
-			{ React.Children.count( children ) > 0 && <BlockSettingsButton /> }
+			{ React.Children.toArray( children ).filter(
+				( child ) => child.props.dontCount !== true


@antonis Thank you for adding the comment above on dontCount. To be honest, though I still had to reread that a few times to understand what the setting was meant to do. I wonder if we can rename it to something like excludeFromSettings or excludeFromCount something along those lines. WDYT?

Suggested change

( child ) => child.props.dontCount !== true

( child ) => child.props.excludeFromSettings !== true

I agree with you @chipsnyder and made the change. I still find this implementation a bit dirty but I couldn't find a better way to achieve this :|

geriux

Hey @antonis 👋 this is looking great! I left a couple of comments, let me know what you think.

packages/block-editor/src/components/inspector-controls/index.native.js

packages/block-library/src/cover/edit.native.js

packages/components/src/color-picker/style.native.scss

lukewalczak

Good job @antonis 🎉 left some small comments!

lukewalczak · 2020-07-23T08:47:34Z

packages/block-library/src/cover/edit.native.js

+							onCustomPress={ () => {
+								if ( isParentSelected ) {
+									openGeneralSidebar();
+									setCustomColorPickerShowing( true );
+								}
+							} }


Could you please extract it to named function?

lukewalczak · 2020-07-23T08:48:56Z

packages/block-library/src/cover/style.native.scss

+	width: 32px;
+	height: 32px;


Suggested change

width: 32px;

height: 32px;

width: $grid-unit-40;

height: $grid-unit-40;

lukewalczak · 2020-07-23T08:50:50Z

packages/components/src/color-palette/index.native.js

+						styles.customIndicatorWrapper,
+						customIndicatorWrapperStyles,


Can we move it to the const since the same styles are used also in other View?

Thank you for reviewing this @lukewalczak :)
I agree with all your suggestions and submitted the changes

This reverts commit 054a6b5.

This reverts commit df25f6b.

geriux · 2020-07-24T10:32:22Z

packages/block-library/src/cover/edit.native.js

+							customVerticalSeparatorStyles={
+								styles.paletteVerticalSeparator
+							}


I think this is not being used, should we remove it or is there something that we need to add?

You are right @geriux ! This was used at some point during the development but not any more.
I updated the PR

geriux

This is working great! Thank you for the changes, just left one minor comment.

geriux

LGTM! Nice work Antonis!! 🎉 Tested it on both iOS and Android.

Antonis Lilis added 3 commits July 9, 2020 18:23

Create cover block with only colour (from a default palette)

c608052

Added custom colour swatch

508f015

Custom Color picker action

96b1582

antonis mentioned this pull request Jul 10, 2020

Cover: Allow creating the block with only color/gradient backgrounds (without an image) wordpress-mobile/gutenberg-mobile#2275

Closed

Hide the block settings button

8c13787

antonis force-pushed the rnmobile/Cover_with_custom_color branch from 6b538b2 to 8c13787 Compare July 10, 2020 14:01

antonis mentioned this pull request Jul 10, 2020

[Mobile] Cover: Add Media Button when only colour is selected #23878

Merged

6 tasks

antonis marked this pull request as ready for review July 10, 2020 16:01

This was referenced Jul 13, 2020

[Mobile] Cover: Adds Clear media button in block settings #23794

Merged

[Mobile] Create cover block with only colour (from a default palette) without an image #23833

Merged

Antonis Lilis added 3 commits July 13, 2020 14:36

Use noop instead of empty arrow function

6e83663

Use default settings colors

3919a7d

WIP: Changes Based on design feedback

a2eee4b

antonis 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 Jul 14, 2020

antonis self-assigned this Jul 14, 2020

antonis linked an issue Jul 14, 2020 that may be closed by this pull request

[Mobile] Cover: Allow creating the block with custom colour (without an image) #23868

Closed

antonis added [Block] Cover Affects the Cover Block - used to display content laid over a background image [Type] Enhancement A suggestion for improvement. labels Jul 14, 2020

Antonis Lilis added 3 commits July 14, 2020 10:52

Merge branch 'master' into rnmobile/23831_Cover_Only_color

83668c7

Merge branch 'master' into rnmobile/23831_Cover_Only_color

c71346a

Merge branch 'rnmobile/23831_Cover_Only_color' into rnmobile/Cover_wi…

a41dc37

…th_custom_color

Antonis Lilis added 6 commits July 14, 2020 16:21

Show palette with both hardcoded and theme colors

0cbe0c1

Customise media placeholder styles only for the cover component

a0cd93b

Merge branch 'rnmobile/23831_Cover_Only_color' into rnmobile/Cover_wi…

30d5ccb

…th_custom_color

Pass only modified media-placeholder styles

7aec919

Pass only modified color-palette styles

a152a4a

Merge branch 'rnmobile/23831_Cover_Only_color' into rnmobile/Cover_wi…

53d5435

…th_custom_color # Conflicts: # packages/block-library/src/cover/style.native.scss # packages/components/src/color-palette/index.native.js

Antonis Lilis added 2 commits July 22, 2020 18:39

Merge branch 'master' into rnmobile/Cover_with_custom_color

0a37400

Design fixes

0a72e9f

chipsnyder reviewed Jul 22, 2020

View reviewed changes

Antonis Lilis added 2 commits July 22, 2020 20:29

Renamed prop to declare usage more clearly

7a4f90d

Lint Issue

8e2c894

geriux reviewed Jul 23, 2020

View reviewed changes

lukewalczak reviewed Jul 23, 2020

View reviewed changes

Antonis Lilis added 9 commits July 23, 2020 13:19

PR review fixes

537211c

Extra fixes to avoid inconsistent states

df25f6b

Lint fix and comment

054a6b5

Revert "Lint fix and comment"

852f9a4

This reverts commit 054a6b5.

Revert "Extra fixes to avoid inconsistent states"

f529dae

This reverts commit df25f6b.

Review changes

a623b1b

Android hardware button handling

b4ab716

Merge branch 'master' into rnmobile/Cover_with_custom_color

979e8d4

Lint fix

a416741

antonis requested review from lukewalczak, geriux and chipsnyder July 24, 2020 09:57

geriux reviewed Jul 24, 2020

View reviewed changes

Removed unused prop/style

2ae97d2

antonis mentioned this pull request Jul 24, 2020

Create cover block with custom colour (without an image) wordpress-mobile/gutenberg-mobile#2508

Merged

2 tasks

geriux approved these changes Jul 24, 2020

View reviewed changes

antonis merged commit bd4a6b7 into WordPress:master Jul 24, 2020

github-actions bot added this to the Gutenberg 8.7 milestone Jul 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mobile] Create cover block with custom colour (without an image) #23870

[Mobile] Create cover block with custom colour (without an image) #23870

antonis commented Jul 10, 2020 •

edited

Loading

antonis commented Jul 10, 2020

iamthomasbishop commented Jul 10, 2020

antonis commented Jul 14, 2020

iamthomasbishop commented Jul 21, 2020

iamthomasbishop commented Jul 21, 2020

antonis commented Jul 22, 2020

iamthomasbishop commented Jul 22, 2020

chipsnyder Jul 22, 2020

antonis Jul 22, 2020

geriux left a comment

lukewalczak left a comment

lukewalczak Jul 23, 2020

lukewalczak Jul 23, 2020

lukewalczak Jul 23, 2020

antonis Jul 23, 2020

geriux Jul 24, 2020

antonis Jul 24, 2020

geriux left a comment

geriux left a comment

	( child ) => child.props.dontCount !== true
	( child ) => child.props.excludeFromSettings !== true

[Mobile] Create cover block with custom colour (without an image) #23870

[Mobile] Create cover block with custom colour (without an image) #23870

Conversation

antonis commented Jul 10, 2020 • edited Loading

Related PRs:

Description

How has this been tested?

Cover block creation with custom colour

Canceling the cover block creation

Screenshots

Types of changes

Checklist:

antonis commented Jul 10, 2020

iamthomasbishop commented Jul 10, 2020

antonis commented Jul 14, 2020

iamthomasbishop commented Jul 21, 2020

iamthomasbishop commented Jul 21, 2020

antonis commented Jul 22, 2020

iamthomasbishop commented Jul 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geriux left a comment

Choose a reason for hiding this comment

lukewalczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geriux left a comment

Choose a reason for hiding this comment

geriux left a comment

Choose a reason for hiding this comment

antonis commented Jul 10, 2020 •

edited

Loading