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

Cover: Add Padding Styles #21492

Merged
merged 106 commits into from Jun 3, 2020
Merged

Cover: Add Padding Styles #21492

merged 106 commits into from Jun 3, 2020

Conversation

ItsJonQ
Copy link
Contributor

@ItsJonQ ItsJonQ commented Apr 8, 2020

Screen Shot 2020-04-08 at 4 46 42 PM

Hi all πŸ‘‹,

Hope you're doing well! I've been working on an exciting update for the Cover block.

This PR adds the ability to add padding styles to the core Cover block. This feature compliments adding internal alignment via Design Tools to improve customization within Gutenberg.

Demo

Screen Capture on 2020-04-08 at 16-18-46

The GIF above demos interacting with the various sides of the new Padding controls. Any changes to Top, Right, Bottom, Left, or All, instantly updates the Cover padding within the Editor's canvas.

New Components

Designing the new controls for padding was an challenging and exciting journey. The goals were:

  • To create an intuitive interface
  • To add quality-of-life interactions with the actual inputs
  • To show updates within the Cover, indicating change

To accomplish these goals, I've put a lot of focus on UI components, specifically on the input elements themselves. Taking in consideration how they behave with the Cover as well as with each other.

Note: The updated/new input/control components supports RTL 🀘

InputControl

The newest, and tiniest, component I've added is a low level InputControl. It's an enhanced version of a bare bones HTML <input>. It's styled out of the box in the spirit of "G2" ([the next advancement of Gutenberg]). More importantly, it has built-in support for "floating labels".

Screen Shot 2020-04-08 at 5 12 33 PM

The "floating label" feature allows us to have (important) labels for the various directions (e.g. "Top") within the condensed space of the sidebar.

NumberControl

InputControl now powers NumberControl!

NumberControl was enhanced to enable drag to increase/decrease values! 😍
This allows for a more organic experience. In the context of Cover, it allows the user to make fluid adjustments without having to use a keyboard.

Speaking of keyboard... holding shift while dragging increases the step by 10!

Just like if you were to press shift + UP or DOWN arrow.

UnitControl

NumberControl powers UnitControl!

BoxControl

Screen Shot 2020-04-08 at 5 19 57 PM

Finally! BoxControl. BoxControl is a generic controller for Top, Right, Bottom, Left values. In this case, we used it for padding. But it could easily be used for margin values as well (if we desire).

Under the hood,BoxControl uses UnitControl for all of the sides.

P.S. BoxControl does support custom units. However, I'm unsure how to fit that experience within the UI right now. Ideas welcome!!

Visualizer

Screen Capture on 2020-04-08 at 16-19-35

BoxControl has an accompanying component called BoxControl.Visualizer. It's designed to flash the side that was most recently changed.

For example, updating the top padding will result in the top flashing ✨

Rendering (CSS)

The Cover has been outfitted with support hooks to render the CSS in the Editor and on the site's front-end. These support hooks are part of the work on global styles.

Testing

This update can be tested in a local Gutenberg development environment.
Be sure to npm install first!
(Potentially lerna bootstrap as well)

Note: At the moment, the padding style values aren't being applied to the Cover block. It was working, but I believe the implementation is not compatible with some recent style hooks changes .

Opt-in

This feature is opt-in via add_theme_support. In order to show the padding controls, the following needs to be added to a theme's function.php

add_theme_support( 'experimental-custom-spacing' );

Thank you so much πŸ™

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.

Related:
#16730

@ItsJonQ ItsJonQ added [Block] Cover [Feature] Design Tools labels Apr 8, 2020
@ItsJonQ ItsJonQ requested a review from mtias Apr 8, 2020
@ItsJonQ ItsJonQ self-assigned this Apr 8, 2020
@ItsJonQ ItsJonQ added the [Status] In Progress label Apr 8, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Apr 8, 2020

Size Change: +4.32 kB (0%)

Total Size: 1.12 MB

Filename Size Change
build/a11y/index.js 1.14 kB +1 B
build/annotations/index.js 3.62 kB +2 B (0%)
build/autop/index.js 2.83 kB -1 B
build/block-directory/index.js 6.48 kB -1 B
build/block-editor/index.js 106 kB +298 B (0%)
build/block-library/index.js 126 kB +277 B (0%)
build/block-library/style-rtl.css 7.69 kB +9 B (0%)
build/block-library/style.css 7.68 kB +5 B (0%)
build/blocks/index.js 48.2 kB -2 B (0%)
build/components/index.js 193 kB +3.6 kB (1%)
build/components/style.css 19.5 kB +4 B (0%)
build/compose/index.js 9.33 kB +11 B (0%)
build/core-data/index.js 11.4 kB -3 B (0%)
build/data-controls/index.js 1.29 kB +2 B (0%)
build/data/index.js 8.45 kB +19 B (0%)
build/date/index.js 5.47 kB +2 B (0%)
build/edit-navigation/index.js 8.06 kB +6 B (0%)
build/edit-post/index.js 302 kB +8 B (0%)
build/edit-site/index.js 14.1 kB +23 B (0%)
build/edit-widgets/index.js 8.89 kB +16 B (0%)
build/editor/index.js 44.7 kB +44 B (0%)
build/format-library/index.js 7.71 kB +3 B (0%)
build/i18n/index.js 3.56 kB +1 B
build/keyboard-shortcuts/index.js 2.51 kB -2 B (0%)
build/keycodes/index.js 1.94 kB +1 B
build/list-reusable-blocks/index.js 3.12 kB -2 B (0%)
build/media-utils/index.js 5.29 kB +2 B (0%)
build/notices/index.js 1.79 kB -2 B (0%)
build/nux/index.js 3.4 kB -3 B (0%)
build/plugins/index.js 2.56 kB -2 B (0%)
build/redux-routine/index.js 2.85 kB -1 B
build/rich-text/index.js 14.8 kB -1 B
build/server-side-render/index.js 2.67 kB -1 B
build/shortcode/index.js 1.69 kB -2 B (0%)
build/token-list/index.js 1.28 kB -1 B
build/viewport/index.js 1.85 kB +9 B (0%)
build/warning/index.js 1.14 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/api-fetch/index.js 3.4 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 787 B 0 B
build/block-directory/style.css 787 B 0 B
build/block-editor/style-rtl.css 11.3 kB 0 B
build/block-editor/style.css 11.3 kB 0 B
build/block-library/editor-rtl.css 7.87 kB 0 B
build/block-library/editor.css 7.88 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 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/components/style-rtl.css 19.5 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.11 kB 0 B
build/edit-navigation/style-rtl.css 857 B 0 B
build/edit-navigation/style.css 856 B 0 B
build/edit-post/style-rtl.css 5.43 kB 0 B
build/edit-post/style.css 5.43 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 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/is-shallow-equal/index.js 711 B 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/url/index.js 4.02 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

loop
src={ url }
/>
<BoxControlVisualizer values={ padding }>
Copy link
Contributor Author

@ItsJonQ ItsJonQ Apr 8, 2020

Choose a reason for hiding this comment

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

Only difference is that the content is contained within <BoxControlVisualizer />

@@ -53,6 +53,7 @@
"re-resizable": "^6.0.0",
"react-dates": "^17.1.1",
"react-spring": "^8.0.20",
"react-use-gesture": "^7.0.9",
Copy link
Contributor Author

@ItsJonQ ItsJonQ Apr 8, 2020

Choose a reason for hiding this comment

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

Leveraged for improved gesture handling. Created by the folks behind react-spring ❀️- an animation library that we currently use within Gutenberg

*/
import { Flex as BaseFlex } from './styles/flex-styles';

function Flex(
Copy link
Contributor Author

@ItsJonQ ItsJonQ Apr 8, 2020

Choose a reason for hiding this comment

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

A layout primitive! Very handy for many common UI cases ✌️
It was used within BoxControl

Copy link
Contributor

@youknowriad youknowriad May 29, 2020

Choose a reason for hiding this comment

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

Do you think we need a README for this?

Copy link
Contributor

@youknowriad youknowriad May 29, 2020

Choose a reason for hiding this comment

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

It's a bit unclear what this gives me over CSS flexbox? Why would I use this instead?

@ItsJonQ ItsJonQ force-pushed the try/cover-padding-control branch from 69bcd37 to 806a615 Compare Apr 9, 2020
@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Apr 9, 2020

@youknowriad / @nosolosw Haii πŸ‘‹

I'm trying to use the new style hooks with these padding controls. It was working a couple of days ago. I just checked and it looked like the inline style CSS variables aren't rendering anymore πŸ€” .

I would love some insight/thoughts!

No rush at all! Thank you! <3

Very excited to be tapping into this style system layer.

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Apr 9, 2020

How does the "Adds dimension controls to Group Block" PR: #16730 padding/margin controls
compare to the PR right here in adding padding styles to the cover block?

I am thinking there should be one margin/padding component/method to be used for all blocks.

@mcsf
Copy link
Contributor

@mcsf mcsf commented Apr 9, 2020

Hi! I'm thinking about the conjunction of this PR with #21091. In particular: why present all four padding controls instead of just two contextual ones?

Let me explain: what if there were two controls that represent "distance to horizontal edge" and "distance to vertical edge"? Then, if the user selects grid position [0, 0], these settings correspond to padding-left and padding-top. Conversely, if the user selects grid position [2, 2], the settings correspond to padding-right and padding-bottom.

When grid position [1, 1] (i.e., centre-middle) is selected, then the user might either see no controls at all, or all four padding controls if they really want that option.

@youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Apr 9, 2020

@ItsJonQ The issue is probably because we moved from CSS variables into inline styles. You should add your inline style mapping here https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/hooks/style.js#L45-L49

@ZebulanStanphill
Copy link
Member

@ZebulanStanphill ZebulanStanphill commented Apr 9, 2020

I think it's also worth noting, for accessibility reasons, the controls ought to be presented in a DOM/tab order that matches their visual order and makes sense. Going by that, the "All" input should come before or after the individual side inputs, rather than in the middle of them.

Personally, I think we should try to promote presets over one-off values, so setting the padding should be similar to setting a font size, where presets are provided alongside a "custom" option.

I think instead of 4 individual side controls + an all sides control, there should be 4 individual side controls + a toggle to lock horizontal values together + a toggle to lock vertical values together.

I also don't think it is necessary to present the options in a fancy cross layout. I think labels like "left" or "horizontal" can convey that just fine, and simplifying the presentation of the controls gives room for a UI more like the font size control to be implemented.

@ItsJonQ ItsJonQ force-pushed the try/cover-padding-control branch from 806a615 to 74883be Compare Apr 14, 2020
@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Apr 14, 2020

The issue is probably because we moved from CSS variables into inline styles. You should add your inline style mapping here https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/hooks/style.js#L45-L49

@youknowriad Ah, thank you so much! I was able to get the inline styles to connect with the controls. However, I noticed an issue...

Since the padding styles are being applied to the block-wrapper (really neat!), it renders to the block container div within the Edit UI, rather than the Cover itself:

Screen Shot 2020-04-14 at 9 10 22 AM

The save output works correctly on the front-end πŸ‘

The only solution that I can think of would involve rendering the values as CSS variables rather than direct style properties. This will enable me to "subscribe" and render those values within the inner Cover. This will work for both Edit and Save.

That being said, I'm open to any suggestions! <3

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Apr 14, 2020

I think it's also worth noting, for accessibility reasons, the controls ought to be presented in a DOM/tab order that matches their visual order and makes sense.

@ZebulanStanphill Thanks for the suggestion! I just made that push.

Originally, I wanted to optimize the tab order to fit the mental model of how directional styles are rendered on the web (top, right, bottom, left). However, I recognize that it's a model that only people who have experience with it would have.

The tab order being tied to visual representation has a much stronger connection πŸ‘

@youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Apr 14, 2020

@ItsJonQ Actually, the best solution here is to refactor the Cover block to use the new lightBlockWrapper flag and the Block.* components in order to match the frontend markup in the editor. The alignments (float, wide...) can be hard to do right though. I also believe it's better to try to refactor the Cover block to lightBlockWrapper in its own PR to avoid mixing with the new additions of this PR. cc @ellatrix not sure if you've refactored blocks like the Cover block yet.

@@ -40,6 +40,9 @@
},
"customGradient": {
"type": "string"
},
"padding": {
Copy link
Member

@oandregal oandregal Apr 14, 2020

Choose a reason for hiding this comment

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

Perhaps we could follow suit on what was done for colors & font-size, so we wouldn't need to declare this attribute per block and declaring support for __experimentalPadding would suffice.

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Apr 14, 2020

Actually, the best solution here is to refactor the Cover block to use the new lightBlockWrapper flag ... I also believe it's better to try to refactor the Cover block to lightBlockWrapper in its own PR...

@youknowriad Sounds good! For the time being, should we go with the CSS variable approach in this PR? Or should we pause on this one to wait for the refactor?

Thanks!

@youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Apr 15, 2020

@ItsJonQ I'd rather try the lighterBlockWrapper first on a separate PR, maybe it's not that hard :)

@oandregal oandregal mentioned this pull request Jun 9, 2020
7 tasks
@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

@nosolosw Thanks for the feedback!! I'll take a looksy πŸ‘€

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

It looks like resetting only works per session, so if a user:

@nosolosw I guess this is a design decision!

Should "Reset" act like a "Clear"? (Where the values reset back to null) or...
Should it reset to the initial value during the session?

I'm okay with either! At the moment, like you pointed out, it's set to reset based on initial session value.

Thanks! <3

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Jun 9, 2020

I think Reset should work to unset any user changes made. Mostly because if that's not the behavior, is there a way for a user to do that?

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

I think Reset should work to unset any user changes made

@jasmussen Gotcha! Thanks for the confirmation :). I'll work on making that happen πŸ’ͺ

is there a way for a user to do that?

At the moment, no. Which I suppose is why we need it to completely reset (or clear) changes :)

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Jun 9, 2020

Good catch, Andres, and thanks for keeping on this Q, it's such a pleasure to watch you work.

@oandregal
Copy link
Member

@oandregal oandregal commented Jun 9, 2020

@ItsJonQ another thing I've just noticed is that the padding attribute is stored directly as a key under style. However, the other style attributes are grouped such as:

  • style.typography: fontSize, lineHeight
  • style.color: link, text, background

If we're preparing the reset change for tomorrow's release, I guess it'd be good to consolidate the padding under the style.spacing namespace? That way, when we add the margin property we already have the namespace and don't need to introduce a block deprecation.

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

@nosolosw Thank you for the suggestion! I think the style.spacing namespace is a great idea

@paaljoachim
Copy link
Contributor

@paaljoachim paaljoachim commented Jun 9, 2020

Feedback:
What I miss is having a way to see the padding as long as the cursor is focused in the spacing panel. Making a change I briefly for a half second see a very light blue color showing the padding. Having this light blue color show longer and atleast as long as the cursor is focused in inside either the panel or just inside the PX box would help to gain a visual view of padding size.

Another example.
When changing the numbers of pixels what about seeing the light blue padding color change size. A few seconds after having changed the size the light blue could disappear.

@ZebulanStanphill
Copy link
Member

@ZebulanStanphill ZebulanStanphill commented Jun 9, 2020

Showing the light blue padding overlay when focused/hovering on the associated input sounds like a neat idea. πŸ‘

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

@nosolosw I have updates! I'm able to adjust the logic to handle reset (null values). Also! For the ability for the user to set zero (0) values, which is different than no user change.

Thanks for the suggestions @ZebulanStanphill + @paaljoachim !
I agree on the highlight / visualizers! I'll see what I can do πŸ’ͺ

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

Screen Capture on 2020-06-09 at 14-37-40

Oh boy oh boy! πŸŽ‰

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Jun 9, 2020

PR here :)
#23041

@rmorse
Copy link
Contributor

@rmorse rmorse commented Sep 8, 2020

I'm using a lot of gutenberg controls on non gutenberg admin pages... This one, I'd like to use in a few places... what about allowing custom icons so we can do something a little like:

image
?

Interested if this is something I should pursue in adding to this, or just copy out the component and customise just for my project.

@ItsJonQ
Copy link
Contributor Author

@ItsJonQ ItsJonQ commented Sep 14, 2020

@rmorse Ooo. That's an interesting idea! πŸ˜„

@mtias
Copy link
Member

@mtias mtias commented Sep 25, 2020

I think we'd want to create some specific versions for some of these properties instead. For example, we have border radius in Button, but it looks nothing like this and we should be consistent internally and in the public component API

@rmorse
Copy link
Contributor

@rmorse rmorse commented Sep 26, 2020

@mtias if you're referring to my mockups - this was just an idea, for my own personal use case - developing admin pages outside of the Gutenberg editor, using Gutenberg components (I guess this is the future workflow for WP admin in general - where theme / plugin authors will be reusing gutenberg components to build their own admin interfaces).

So what I want to be able to do is allow each dimension to be controlled individually, ( ie, the border radius properties ), rather than be "stuck" with Gutenberg best practise as in your example the radius control, because my use case is different - and this is a great component I could be repurposing.

I of course agree that it's necessary to keep consistency / good practice patterns within the Gutenberg editing experience, however I would love to keep the kind of flexibility that choosing custom icons would provide for this component, and as a general pattern / approach it would be great to bear in mind flexibiliy with components for developers as a whole, who will be building custom UIs now, and even more so further down the line using Gutenberg components

-I'm not sure you are actually suggesting to limit this flexibility... but wanted to share my perspective anyway :)

I'm nearly finished working on a plugin with a custom admin UI, and overall its been a fantastic experience dropping in Gutenberg components all over the place, saving me countless hours and making it 100x more accessible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment