Add visual indicator if a block is connected to block binding source #59185
Conversation
michalczaplinski
added
[Type] Bug
|
Size Change: +1.61 kB (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 06fdbf7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8009112699
|
|
The release beta cycle might not be a good time to introduce a new public SlotFill. We can use a private SlotFill (see example #49819) or add a component directly to the toolbar like |
This is the expected change in the toolbar that triggered refactoring to introduce a new slot:
@SaxonF or @jasmussen, what would be the alternative to change the block's icon instead in a way that retains the original but also gives a clear indicator that it's connected? This way, we wouldn't have to introduce another toolbar group. In its current shape, should it even be a toolbar item? Is it actionable? Anyway, the other aspect is that we should stop using the filters to integrate Block Bindings with the block editor, but instead consider it part of the framework that should be tightly integrated. In effect, the changes should go directly to the block toolbar to add icon indicator there. The border for the block's content can be integrated withing |
We will need to cherry-pick this PR to the |
Functionally this seems okay in the name of shipping the work. That being said, it feels like there's potential for a deeper design iteration going forward, since the feature is so potent, so this is something we'll want to follow up on. The issue here goes back to the same issue we have elsewhere in the site editor, more clearly delineating what content is reusable/global, and what content is local. We haven't fully threaded the needle on how to address that; the purple sync color is a useful start, the "Content" panel and some of the work around pattern overrides can potentially be leveraged here as well. But ideally we can find one concept that would work across all of those, and make it much clearer for anyone editing, where and what they are changing. |
|
Thanks for the feedback folks! I've moved the implementation directly to the I've tried to use the private SlotFills but I wasn't quite able to make it work. Unlike the 
|
packages/block-editor/src/components/block-bindings-button/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-bindings-button/index.js
Outdated
Show resolved
Hide resolved
|
Wondering if we could highlight properties in the toolbar that are connected much like we will do in inspector where we use an icon. In toolbar we could highlight with colour + shape. If a block's "primary" attribute is connected we'd highlight the block icon e.g. a paragraph content.
So for an image that has src, link and alt text connected when in a template it would look something like
|
afercia
added
the
[Focus] Accessibility (a11y)
|
Any important information need a visual indicaotr but also needs to be provided in a semantic and accessible way. I'm not sure a button that doesn't do anything and its only used for visual purposes is ideal.
Overall, I'm not sure the toolbar is the right place to add an indicator to. I'd see the Settings panel a sa more appropriate palce, as that's where block properties and settings are available to users. |
|
Interesting concept @SaxonF, thanks! That does look better visually IMO. My only concern is that we would have to use custom fields-specific logic to handle this. Sources other than custom fields might not have fields whose names we can display here. 
What if it just says "Connected to |
Actually, scratch that because each of block's attributes can be connected to multiple sources so that won't work either 🙁 . What we'd need is a way to indicate the "Connected to " in the UI per attribute |
|
Hi @afercia I understand some of your concerns. About the "button": We could get rid of the
That's a subjective opinion about what the UI should look like so it's hard to argue one way or the other 🙂
I've changed it to just "Connected" in bbd3fcc
I'm not sure I understand - It's not the only indicator, it's in addition to the indicators that are already present:
So, it's an improvement over the status quo.
That's again a subjective opinion about what UI should look like 🙂 Besides, e.g. the Button block has a parent selector as the first item in the toolbar.
As of right now, there is no indication of the state so I don't understand how adding dots would be worse? The design proposed by @SaxonF also adds indicators of "connected" state in the block Settings as you suggest. Is that what you're proposing? |
|
RE: Avoiding rendering a
|
michalczaplinski
requested review from
tellthemachines and
ellatrix
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
That's a good catch @SantosGuillamot . In addition to checking |
@michalczaplinski Addressed this in ac72c0a |
packages/block-editor/src/components/block-bindings-toolbar-indicator/index.js
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| .block-editor-block-bindings-indicator svg g { | ||
| stroke: #9747ff; |
There was a problem hiding this comment.
@jasmussen or @SaxonF - do you usually put colors that are used in several places under a variable name?
There was a problem hiding this comment.
-
Should we reuse this property ?
-
Currently the color of the "bound" blocks is
#9747ffwhich is close but different from synced blocks color (#7a00df). Should it be the same?
There was a problem hiding this comment.
OK, in 4dc1103, I've added a new --wp-bound-block-color property to _default-custom-properties.scss that I reused in other components.
We can still change this color in the future.
There was a problem hiding this comment.
I'd defer to Saxon, but my instinct would've been to make these the same, so it could be good to follow up and retire the variable. We do reuse colors like this, there should almost never be stray hex codes in the codebase. But at the same time we're also very careful with introducing new variables. Sorry for the delayed response @michalczaplinski
- get rid of 2 unnecessary `<path>` elements - move the stroke styles to CSS - add the `evenodd` rule
|
I'm pretty happy with the PR now. If there aren't any more concerns I will merge later today :) |
|
Not sure why "some checks were not successful" because looking at the individual checks they all seem to pass. I will merge now as this seems like a github reporting issue. There's one last thing I've noticed, the block icon color in the List View is barely visible if the block is selected: 
We can fix this in a quick follow-up PR. |
|
It seems that when I click on a block that is connected it throws an error in the console: @michalczaplinski @artemiomorales Could you check if you are able to reproduce it and, in that case, fix it please? |
@SantosGuillamot Fixed in the following commit on the same PR to fix the fill color. |
…59185) * Add BlockControlsFirst slot to block controls groups * Add connection icon to BlockControls toolbar button * Add block binding toolbar button if block is connected to a source * Add i18n support for block toolbar button label * Add BlockBindingsButton component and remove BlockControlsFirst group * Refactor BlockBindingsButton to check for block connections * Change the ToolbarButton label * Update block-bindings-button import to block-bindings-indicator * Block Bindings: Add connection icon to list view (#59331) * Add connection icon to list view * Remove extraneous string * Move bindings style to useBlockProps * Remove extraneous comment * Move bindings selector logic to toolbar * Rename indicator file * Move purple stroke style from SVG markup to CSS * Check if block can be bound before adding styles * Simplify the SVG icon: - get rid of 2 unnecessary `<path>` elements - move the stroke styles to CSS - add the `evenodd` rule * Update the CSS namespacing to include the `__` * Fix issues with block binding indicator color --------- Co-authored-by: michalczaplinski <czapla@git.wordpress.org> Co-authored-by: artemiomorales <artemiosans@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org>
|
I just cherry-picked this PR to the update/packages-6.5-rc1 branch to get it included in the next release |
youknowriad
added
Backported to WP Core
…59185) * Add BlockControlsFirst slot to block controls groups * Add connection icon to BlockControls toolbar button * Add block binding toolbar button if block is connected to a source * Add i18n support for block toolbar button label * Add BlockBindingsButton component and remove BlockControlsFirst group * Refactor BlockBindingsButton to check for block connections * Change the ToolbarButton label * Update block-bindings-button import to block-bindings-indicator * Block Bindings: Add connection icon to list view (#59331) * Add connection icon to list view * Remove extraneous string * Move bindings style to useBlockProps * Remove extraneous comment * Move bindings selector logic to toolbar * Rename indicator file * Move purple stroke style from SVG markup to CSS * Check if block can be bound before adding styles * Simplify the SVG icon: - get rid of 2 unnecessary `<path>` elements - move the stroke styles to CSS - add the `evenodd` rule * Update the CSS namespacing to include the `__` * Fix issues with block binding indicator color --------- Co-authored-by: michalczaplinski <czapla@git.wordpress.org> Co-authored-by: artemiomorales <artemiosans@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org>
What?
Add visual indicators if a block is connected to block bindng sources:
#9747FFPart of fixes in #58978.
Why?
To make the "bound" blocks visually distinct.
How?
@wordpress/iconsTesting Instructions
Register a new custom field however you prefer. You can use a snippet similar to this:
In all of the following cases, verify that the purple focus outline is visible and that the connection icon appears both in the block toolbar and the list view.
Test paragraph
Add a paragraph with the content connected to a custom field:
Test heading
Repeat the paragraph test but using a heading.
Test button
Add a button with the content connected to a custom field:
Screenshots or screencast