Add ability to filter the settings options for <LinkControl>
#32150
Conversation
getdave
added
[Type] Experimental
|
Size Change: +58 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
Looks like if this goes ahead, I'll need to update some of the e2e tests. That should be simple enough but I won't do it until I get a 🟢 light on direction. |
gziolo
added
the
[Feature] Extensibility
|
There was a previous attempt - #13190 and I see there's an issue as well #11599 I can see this makes it possible to add settings to the link control, but I'm not sure how an implementer using the filter could then do something with those settings. For example, if I wanted to add an additional setting to the button block's link attributes, how could that be serialised to the block's markup? I remember some previous discussion on this that mentioned that it's hard to introduce very generic extensibility when the LinkControl is used in such different ways across the codebase. There's the case of the link format, which serialises its data using the format api. Then specific blocks which use attributes. And there might even be use of the component as general UI outside of blocks or formats. It's unclear to me if a plugin developer would want global extensibility, or only extensibility of a specific usage of LinkControl. Or possibly both? I think there's also a question over whether every setting should be a boolean as well. It might be better as a slot/fill so there's more freedom for using a variety of form components. |
|
I have an idea. Let me work on it here. |
|
A slot would definitely have use cases, eg allowing custom |
getdave
force-pushed
the
add/filter-link-control-settings
branch
from
7e5d4ed to
41b18e0
Compare
|
Hi @talldan. I took another look at this.
That's good.
As you know the link control settings drawer component calls an However, the issue is that developers using that filter don't have control of the For example below it's trivial to add the functionality you require to the Button block... diff --git a/packages/block-library/src/button/edit.js b/packages/block-library/src/button/edit.js
index 15235369b2..82f3734775 100644
--- a/packages/block-library/src/button/edit.js
+++ b/packages/block-library/src/button/edit.js
@@ -7,7 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
-import { useCallback, useState, useRef } from '@wordpress/element';
+import { useCallback, useState, useRef, useEffect } from '@wordpress/element';
import {
Button,
ButtonGroup,
@@ -32,6 +32,20 @@ import { createBlock } from '@wordpress/blocks';
const NEW_TAB_REL = 'noreferrer noopener';
+wp.hooks.addFilter(
+ 'core.block-editor.link-control.settings',
+ 'myplugin/block-editor/link-control-settings',
+ ( settings ) => {
+ return [
+ ...settings,
+ {
+ id: 'noFollow',
+ title: 'No follow',
+ },
+ ];
+ }
+);
+
function WidthPanel( { selectedWidth, setAttributes } ) {
function handleChange( newWidth ) {
// Check if we are toggling the width off
@@ -70,6 +84,7 @@ function URLPicker( {
url,
setAttributes,
opensInNewTab,
+ noFollow,
onToggleOpenInNewTab,
anchorRef,
} ) {
@@ -88,6 +103,7 @@ function URLPicker( {
} );
setIsURLPickerOpen( false );
};
+
const linkControl = ( isURLPickerOpen || urlIsSetandSelected ) && (
<Popover
position="bottom center"
@@ -96,13 +112,14 @@ function URLPicker( {
>
<LinkControl
className="wp-block-navigation-link__inline-link-input"
- value={ { url, opensInNewTab } }
+ value={ { url, opensInNewTab, noFollow } }
onChange={ ( {
url: newURL = '',
opensInNewTab: newOpensInNewTab,
+ noFollow: newNoFollow,
} ) => {
- setAttributes( { url: newURL } );
-
+ console.log( 'no follow is:', newNoFollow );
+ setAttributes( { url: newURL, noFollow: newNoFollow } );
if ( opensInNewTab !== newOpensInNewTab ) {
onToggleOpenInNewTab( newOpensInNewTab );
}
@@ -164,6 +181,7 @@ function ButtonEdit( props ) {
text,
url,
width,
+ noFollow,
} = attributes;
const onSetLinkRel = useCallback(
( value ) => {
@@ -210,6 +228,7 @@ function ButtonEdit( props ) {
[ `has-custom-font-size` ]: blockProps.style.fontSize,
} ) }
>
+ { noFollow && <p>I can haz no follow!</p> }
<RichText
aria-label={ __( 'Button text' ) }
placeholder={ placeholder || __( 'Add text…' ) }
@@ -245,6 +264,7 @@ function ButtonEdit( props ) {
isSelected={ isSelected }
opensInNewTab={ linkTarget === '_blank' }
onToggleOpenInNewTab={ onToggleOpenInNewTab }
+ noFollow={ noFollow }
anchorRef={ ref }
/>
<InspectorControls>
....but you have to know the name of the setting in advance and thus each new setting requires amendments to the button block. This is the same for rich text and everywhere else the LinkControl is used. SolutionI wonder whether we ought to make onChange( {
...value,
- [ setting.id ]: newValue,
+ settings: {
+ ...value.settings,
+ [ setting.id ]: newValue,
+ },
} );I've made this change just so we can see how it might work. It would obviously have a number of knock on effects, but I'm wondering if we could provide an API to simplify the process of serializing all the settings to the |
|
It looks like this thread has died off over the last couple of months, but I think this PR is worthwhile to resurrect and get merged into core. Plugin authors are extending the link control in any case, but are forced to go about it in the wrong way because of the limitations. Take Rank Math SEO plugin as an example, which adds a bunch of rel nofollow attributes onto links for SEO purposes. The way they get around this is by deregistering wplink and registering their own version (https://plugins.trac.wordpress.org/browser/seo-by-rank-math/trunk/includes/admin/class-assets.php#L180). Surely this will lead to potential major problems down the line? Having the ability to hook in and extend the link control is both needed and long overdue IMHO |
|
This approach won't work but I have another plan which I will link to here shortly. |
|
@getdave any news on this ? This is kind of critical for a number of things like adding aria-label for accessibility, adding obfuscation for seo, adding nooppener noreferrer nofollow etc |
|
There's an experiment over at #39768 which I have not found time to take further. Essentially we can't just expose these settings without making autohandling of persisting the values in the UI work. Otherwise we'll end up with |
|
Any news on this? |
|
Whilst I appreciate it's still something folks want to achieve this has necessarily fallen down my list of priorities. I'd really like to find a way to get back to it though. Contributions are also very much welcomed. |
|
@getdave Ok, unfortunate - but understandable. Appreciate the update though. 🫶 Any general direction how this should proceed from your point of view? |
Essentially the issue is that the I started experimenting with the idea of allowing the developer to define their own functions which define how to convert to/from the This would require both
It was pretty complex and I'm not sure it will work. I didn't get a chance to look into alternative solutions. |
|
Would love the ability to extend the link control popover or add settings. Specifically, I have strict requirements for what needs to be read out for screen readers, and sometimes we have things like image links. |
Description
As suggested by Tammie, this PR adds a filter to the
<LinkControlSettingsDrawer>component to allow Plugin developers to customise the options displayed there.There is some concern regarding adding too much visual "clutter" to such an important piece of UI. Therefore, this implementation intentionally does not use Slot/Fill and prefers the hooks API (yes I am aware that we prefer not to use filters in Gutenberg) in order to limit the scope of what can be added. Currently, this is limited to boolean-type options only.
It was suggested to add a toggle to show/hide the additional options based on the number of options shown. THis was to avoid undue visual clutter. This PR does not attempt to address this aspect.
Note: I am suggesting this filter is marked as
__experimentalin order that it can be revoked if the community feel it is becoming a problem. I haven't yet implemented it as such but that can't be changed very easily by updating the hook name from:...to something like:
cc'ing @mtias as I have a feeling he had some opinions on whether this is a good idea.
How has this been tested?
I've added automated unit tests to validate the filter.
You can also test the UI directly using browser dev tools.
Consoletab of dev tools.Screenshots
Screen.Capture.on.2021-05-24.at.12-26-24.mp4
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).