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.

Sign up for GitHub

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

Jump to bottom

PLANET-4924 Re-enable the native buttons block #251

Merged
merged 2 commits into from
Apr 21, 2020
Merged

PLANET-4924 Re-enable the native buttons block #251

merged 2 commits into from
Apr 21, 2020

Conversation

sagarsdeshmukh
Copy link
Member

@sagarsdeshmukh sagarsdeshmukh commented Apr 10, 2020 •
edited
Loading

The button block is updated in WP-5.4 release with "buttons" block. The new buttons block offers additional options to editors -

  • Allow to set custom button border radius (finding a way to disable this option...)
  • New button style options(we disabled it and allow P4 buttons styles only)
  • Color gradient option for button background
  • Toggle option to "Open in new tab" (this options seems useful)

Task -

  • Disable border radius option
  • Allow only the colours mentioned in Will's comment for colour palette( JIRA 4924 )
  • Ensure that it still follows our custom styles
  • Whitelist buttons block
  • Disable the gradient presets & custom gradients

The WP core buttons block,wont have a build in setting to disable the border radius option and update custom text and background color. The issue regarding build in support to control button block is open( Issue #19796 )

The approach I followed here to fix button block is, Override core button block edit method with our requirement. (ref. Issue #19796 comment)

Eg. https://k8s.p4.greenpeace.org/test-titan/test-button-block/

Styleguide PR: greenpeace/planet4-styleguide#47

@comzeradd comzeradd changed the base branch from develop to master April 10, 2020 09:08
@sagarsdeshmukh sagarsdeshmukh added Review and removed WIP labels Apr 16, 2020
@sagarsdeshmukh sagarsdeshmukh force-pushed the planet-4924 branch 2 times, most recently from b29acd0 to 1158f1e Compare April 17, 2020 07:30
sagarsdeshmukh
sagarsdeshmukh commented Apr 17, 2020
View reviewed changes
classes/class-loader.php
@@ -570,6 +570,10 @@ public function set_color_palette() {

// Disable custom color option.
add_theme_support( 'disable-custom-colors' );

// Disable gradient presets & custom gradients.
add_theme_support( 'editor-gradient-presets', [] );
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: Here I disabled the gradient presets & custom gradients globally.

@sagarsdeshmukh sagarsdeshmukh mentioned this pull request Apr 17, 2020
Merged
@sagarsdeshmukh sagarsdeshmukh requested review from a team, pablocubico and Inwerpsel and removed request for a team April 20, 2020 03:55
@sagarsdeshmukh sagarsdeshmukh force-pushed the planet-4924 branch 2 times, most recently from 476dcfa to 4e43256 Compare April 20, 2020 04:38
Inwerpsel
Inwerpsel reviewed Apr 20, 2020
View reviewed changes
Copy link
Contributor

@Inwerpsel Inwerpsel left a comment

Choose a reason for hiding this comment

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

Can you update the link to the gutenberg file you based on to point to the right commit/tag?

assets/src/BlockFilters.js Outdated
return settings;
}

if ( settings.name = 'core/button' ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be ===

assets/src/p4_button/edit.js Outdated
@@ -0,0 +1,273 @@
/**
* This file is copy of core button block edit.js (https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/button/edit.js), with customize changes.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for including the url here. Could you just have it point to the exact commit that our version is based on. If it points to blob/master this will not match anymore once the file is changed in Gutenberg.

Inwerpsel
Inwerpsel reviewed Apr 21, 2020
View reviewed changes
assets/src/components/p4_button/edit.js Outdated
@@ -57,31 +75,6 @@ const applyFallbackStyles = withFallbackStyles( ( node, ownProps ) => {
} );

const NEW_TAB_REL = 'noreferrer noopener';
const MIN_BORDER_RADIUS_VALUE = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to keep this part in, as we already removed its usage. Keeping it will make it a lot easier to keep this file up to date with whatever changes happen in Gutenberg.

Inwerpsel
Inwerpsel approved these changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@Inwerpsel Inwerpsel left a comment

Choose a reason for hiding this comment

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

Nice work! Maybe you can re-add the BorderPanel definition and the constants? Even though they're not used anymore with your changes, it's probably better to keep this file as close to the original as possible to make it easier to keep up to date.

@sagarsdeshmukh sagarsdeshmukh force-pushed the planet-4924 branch 3 times, most recently from bb5925f to 216cfd7 Compare April 21, 2020 08:46
@sagarsdeshmukh sagarsdeshmukh merged commit ea226b7 into master Apr 21, 2020
@sagarsdeshmukh sagarsdeshmukh deleted the planet-4924 branch April 21, 2020 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Inwerpsel Inwerpsel Inwerpsel approved these changes

@pablocubico pablocubico Awaiting requested review from pablocubico

Assignees
No one assigned
Labels
Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sagarsdeshmukh @Inwerpsel