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

Separator: Add borderWidth style support #23403

Closed
wants to merge 1 commit into from

Conversation

ItsJonQ
Copy link

@ItsJonQ ItsJonQ commented Jun 23, 2020

This update adds a control for border width for the Separator block. This is achieved using the new styled hook/supports registration system and CSS variables.

Screen Capture on 2020-06-23 at 15-10-44

Above is a GIF demo of the control working.

Note: How Separator appears to be highly dependant on a theme's CSS. For example, TwentyTwenty uses very custom CSS in order to achieve the separator designs. In order to demo this, I had to adjust a CSS value (via Chrome inspect element) to use the variable.

CSS Variable

The CSS variable for the separator border is:

var(--wp--style--border--width);

Opt-in

To opt into this feature, one must add:

add_theme_support( "experimental-custom-border" );

Feedback

cc'ing @mtias 😊 . I'd love for some feedback! Does adding this control under a "Border" panel make sense? Also... I'm unsure about the way <hr /> are rendered by existing CSS. It seems like most users won't see the effect, unless they make some adjustments in their WordPress theme 🤔

How has this been tested?

  • Add the add_theme_support in PHP
  • Run npm run dev
  • Add a Separator block
  • (Maybe adjust some CSS in the browser, depending on your theme)
  • Adjust the value of the Border Width

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.

Hopes to resolve:
#20758

@ItsJonQ ItsJonQ added [Status] In Progress Tracking issues with work in progress [Block] Separator Affects the Separator Block labels Jun 23, 2020
@ItsJonQ ItsJonQ requested a review from mtias June 23, 2020 19:26
@ItsJonQ ItsJonQ self-assigned this Jun 23, 2020
height: 28,
lineHeight: '24px',
Copy link
Author

Choose a reason for hiding this comment

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

Adjustments to label height. Abstracted from:
#23006

@github-actions
Copy link

Size Change: +345 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.js 107 kB +224 B (0%)
build/block-library/index.js 129 kB +14 B (0%)
build/block-library/style-rtl.css 8.04 kB +12 B (0%)
build/block-library/style.css 8.05 kB +13 B (0%)
build/block-library/theme-rtl.css 774 B +44 B (5%) 🔍
build/block-library/theme.css 775 B +43 B (5%) 🔍
build/components/index.js 197 kB -13 B (0%)
build/editor/index.js 44.9 kB +8 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.27 kB 0 B
build/block-directory/style-rtl.css 937 B 0 B
build/block-directory/style.css 937 B 0 B
build/block-editor/style-rtl.css 10.6 kB 0 B
build/block-editor/style.css 10.6 kB 0 B
build/block-library/editor-rtl.css 7.6 kB 0 B
build/block-library/editor.css 7.6 kB 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/blocks/index.js 48.2 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.62 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/index.js 9.87 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.48 kB 0 B
build/edit-post/style.css 5.47 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.02 kB 0 B
build/edit-site/style.css 3.02 kB 0 B
build/edit-widgets/index.js 9.33 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 663 B 0 B
build/nux/style.css 660 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.67 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.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@paaljoachim
Copy link
Contributor

I just want to say that this is super helpful to be able to adjust the width (height) of the line or dots used. As the default line might be too thin and having the option to make it bigger helps.

@mtias
Copy link
Member

mtias commented Oct 9, 2020

@ItsJonQ should this be border-width or just height?

@paaljoachim
Copy link
Contributor

It would be more natural to call it height. As in how tall the line is. - Vertical.
As the width affects the left/right area. As in how wide is the line. - Horizontal.

So let's say there was a control named height and another control named width. The names in themselves would then make it clear that they do. Height would make the line taller. Width would make the line broader. Expect further in horizontal directions.

Another scenario with another control.
Let's say that one adds alignment. Left, center and right. User selects to have the line to the left. With a 50% width, and a 5 pixel tall line.

Bottom line is that the Separator block can then have additional controls affecting where on the page it is located (left/center/right), how wide it is, and how tall it is. Hmm I might just have to add an issue or two or so for this. There is some work on Separator styles going on in WordPress/twentytwentyone#278

@paaljoachim
Copy link
Contributor

paaljoachim commented Oct 9, 2020

Btw Jon @ItsJonQ

Check out this comment by @richtabor
#16483 (comment)
As in how in the animated example shows an adjusting of the height of the block making it feel more similar to the Spacer block. NB. The example just adjusts the height of the block so one does not need to add a Spacer block above/below.
It does not affect the height/thickness of the line.

@ItsJonQ
Copy link
Author

ItsJonQ commented Oct 9, 2020

@ItsJonQ should this be border-width or just height?

@mtias I think "size" may be the clearer (and simpler) term to go width 😅 . The CSS property is "width", but visually, we see "height". Size applies to both 😂

@ItsJonQ
Copy link
Author

ItsJonQ commented Oct 9, 2020

It's been a while since this PR was opened, and it looks like a lot has happened with the theme.json / block library settings.
I think it would be easier to close this PR + branch, and start a new 😊

@simison
Copy link
Member

simison commented Nov 6, 2020

"Thickness" as the setting name might work well as well, because "height" could imply the height of the whole area the block takes (including empty space).

Base automatically changed from master to trunk March 1, 2021 15:43
@scruffian scruffian mentioned this pull request May 17, 2022
23 tasks
@draganescu
Copy link
Contributor

draganescu commented May 23, 2022

I picked this up and so far adding

		"__experimentalBorder": {
			"color": true,
			"radius": true,
			"style": true,
			"width": true,
			"__experimentalDefaultControls": {
				"color": true,
				"radius": true,
				"style": true,
				"width": true
			}
		}

to block.json brings this:

border-separator.mp4

The rendering has to also be updated to account for the borders.

@youknowriad
Copy link
Contributor

Doing some repository cleanup: Closing this PR as it probably needs to be redone entirely using block support anyway.

@youknowriad youknowriad deleted the add/separator-block-border-width-controls branch April 25, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Separator Affects the Separator Block [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants