Update `LinkControl` component to utilitse dynamic settings for additional settings "drawer" #18285

getdave · 2019-11-05T13:04:23Z

The new experimental Link UI provided by LinkControl allows for a currently selected link to have settings such as "Open in new tab"...etc

As things stand the only setting that is possible is "new tab" as this has been hard coded.

This PR updates so that multiple settings can be provided by passing the currentSettings prop to LinkControl:

<LinkControl
	// ...other props here
	currentSettings={ [
		{
			id: 'newTab',
			title: 'Open in New Tab',
			checked: false,
		},
		{ // this is custom!
			id: 'noFollow',
			title: 'No follow',
			checked: true,
		},
	] }
/>

How has this been tested?

Automated tests cover this change.

Types of changes

Breaking change (fix or feature that would cause existing functionality to not work as expected).

As this updates LinkControl we can expect this PR to break. It will need updating so that:

new-tab becomes newTab in any settings provided.

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.

getdave · 2019-11-05T13:07:29Z

@phpbits This is relevant to #17557 (comment)

getdave · 2019-11-05T13:14:46Z

@retrofox I'm interested in how you see my change working with https://github.com/WordPress/gutenberg/pull/18062/files#diff-17e2993cb0b8cf64434015038b235162R214

retrofox

Testing

I've tested locally in my dev env. It looks pretty good!

It's pretty easy to use and narrows very good with the block attributes:

<LinkControl
	currentSettings={ [
		{
			id: 'opensInNewTab',
			title: __( 'Open in New Tab' ),
			checked: opensInNewTab,
		},
		{
			id: 'noFollow',
			title: 'No follow',
			checked: noFollow,
		},
	] }
	onSettingsChange={ ( setting, value ) => setAttributes( { [ setting ]: value } ) }
/>

retrofox · 2019-11-05T19:54:55Z

btw feel free to go-ahead not waiting for #18062 👍

getdave · 2019-11-06T11:35:43Z

I'll be rebasing this branch shortly and tweaking the visual styles. Then we're 👍 to merge.


        Adds basic coverage test for new tab setting


        Refactor ‘new-tab` to camelCase for consistency


        Fix bug with null render if settings has a length and convert to array

Make settings an array so that it can be ordered. Fix incorrect conditional testing for .length to ensure it passes if settings _does_ have a length.


        Updates to loop over supplied settings

Previously only a “new tab” setting was hardcoded. Update so retain “new tab” as the default, but also allow for custom settings if/when provided.


        Updates docs


        Updates Nav Item Block to utilise new setting format


        Updates spacing between individual setting controls

getdave · 2019-11-06T12:20:01Z

@retrofox Having to pass the entire config object even to use the default setting feels like a lot of effort:

<LinkControl
	currentSettings={ [
		{
			id: 'opensInNewTab', // why can't I omit this?
			title: __( 'Open in New Tab' ), // why can't I omit this?
			checked: opensInNewTab, 
		},
	] }
	onSettingsChange={ ( setting, value ) => setAttributes( { [ setting ]: value } ) }

Is there a way to make this API nicer?


        Update docs for settings

retrofox · 2019-11-06T12:39:41Z

While I was testing I immediately thought that the id and checked properties could be combined in just one. At least it could be optional:
If the checked is not defined we can try to apply the value through of the id

<LinkControl
	currentSettings={ [
		{
			id: 'opensInNewTab',
			title: __( 'Open in New Tab' ), // why can't I omit this?
		},
	] }

About the title, maybe? I guess that maybe it could be handled by the parent component? Not sure, though. We can just remove the title if it isn't defined.

retrofox · 2019-11-06T13:46:48Z

Dave, I think we should update how the Menu Item updates the link settings: something like that?

const updateLinkSetting = ( setter ) => ( setting, value ) => {
	setter( { [setting]: value } );
};

retrofox · 2019-11-06T13:48:59Z

Also, we will need to update the setting ID here to opensInNewTab


        Updates setting key/id to match attribute to simply and improve consi…

…stency

getdave · 2019-11-06T15:03:06Z

Ok @retrofox this is updated.

packages/block-library/src/navigation-menu-item/edit.js


        Fix Nav Menu Block dynamic setting of attribute

Addresses #18285 (comment)


        Update `LinkControl` component to utilitse dynamic settings for addit…

…ional settings "drawer" (WordPress#18285) * Adds basic coverage test for new tab setting * Refactor ‘new-tab` to camelCase for consistency * Fix bug with null render if settings has a length and convert to array Make settings an array so that it can be ordered. Fix incorrect conditional testing for .length to ensure it passes if settings _does_ have a length. * Updates to loop over supplied settings Previously only a “new tab” setting was hardcoded. Update so retain “new tab” as the default, but also allow for custom settings if/when provided. * Updates docs * Updates Nav Item Block to utilise new setting format * Updates spacing between individual setting controls * Update docs for settings * Updates setting key/id to match attribute to simply and improve consistency * Fix Nav Menu Block dynamic setting of attribute Addresses WordPress#18285 (comment)

getdave added the [Package] Block editor label Nov 5, 2019

getdave requested review from retrofox, draganescu and talldan Nov 5, 2019

getdave requested review from ellatrix and youknowriad as code owners Nov 5, 2019

getdave self-assigned this Nov 5, 2019

getdave added this to 👀 PRs to review in Navigation block via automation Nov 5, 2019

getdave requested a review from obenland Nov 5, 2019

getdave mentioned this pull request Nov 5, 2019

NavigationMenu: <LinkControl /> integration. #18062

Merged

12 of 12 tasks complete

getdave requested review from frontdevde and marekhrabe Nov 5, 2019

retrofox approved these changes Nov 5, 2019

View changes

getdave added 5 commits Nov 5, 2019

Adds basic coverage test for new tab setting

ab84e3d

Refactor ‘new-tab` to camelCase for consistency

268da19

Fix bug with null render if settings has a length and convert to array

63214ff

Make settings an array so that it can be ordered. Fix incorrect conditional testing for .length to ensure it passes if settings _does_ have a length.

Updates to loop over supplied settings

ba312a9

Previously only a “new tab” setting was hardcoded. Update so retain “new tab” as the default, but also allow for custom settings if/when provided.

Updates docs

Loading status checks…

0458a32

getdave force-pushed the update/link-control-utilitse-dynamic-settings branch from ae9da61 to 0458a32 Nov 6, 2019

Updates Nav Item Block to utilise new setting format

Loading status checks…

b40ce8b

getdave requested review from ajitbohra, jorgefilipecosta and Soean as code owners Nov 6, 2019

Updates spacing between individual setting controls

Loading status checks…

d0b17d1

Update docs for settings

Loading status checks…

de238a6

Updates setting key/id to match attribute to simply and improve consi…

Loading status checks…

c3e553f

…stency

retrofox reviewed Nov 6, 2019

View changes

packages/block-library/src/navigation-menu-item/edit.js Outdated Show resolved Hide resolved

Fix Nav Menu Block dynamic setting of attribute

Loading status checks…

b194fe1

Addresses #18285 (comment)

getdave merged commit 3ab9fb6 into master Nov 6, 2019
2 checks passed

2 checks passed

pull-request-automation
Details

Travis CI - Pull Request Build Passed
Details

Navigation block automation moved this from 👀 PRs to review to ✅ Done Nov 6, 2019

getdave deleted the update/link-control-utilitse-dynamic-settings branch Nov 6, 2019

youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019

Nov	DEC	Jan
	11
2018	2019	2020

WordPress/gutenberg

Update `LinkControl` component to utilitse dynamic settings for additional settings "drawer" #18285

Update `LinkControl` component to utilitse dynamic settings for additional settings "drawer" #18285

getdave commented Nov 5, 2019 •

edited

This comment has been minimized.

getdave commented Nov 5, 2019

This comment has been minimized.

getdave commented Nov 5, 2019

retrofox left a comment

This comment has been minimized.

retrofox commented Nov 5, 2019

This comment has been minimized.

getdave commented Nov 6, 2019

This comment has been minimized.

getdave commented Nov 6, 2019 •

edited

This comment has been minimized.

retrofox commented Nov 6, 2019

This comment has been minimized.

retrofox commented Nov 6, 2019

This comment has been minimized.

retrofox commented Nov 6, 2019

This comment has been minimized.

getdave commented Nov 6, 2019

WordPress/gutenberg

Perfect your code

Update `LinkControl` component to utilitse dynamic settings for additional settings "drawer" #18285

Update `LinkControl` component to utilitse dynamic settings for additional settings "drawer" #18285

Conversation

getdave commented Nov 5, 2019 • edited

How has this been tested?

Types of changes

Checklist:

This comment has been minimized.

getdave commented Nov 5, 2019

This comment has been minimized.

getdave commented Nov 5, 2019

retrofox left a comment

Testing

This comment has been minimized.

retrofox commented Nov 5, 2019

This comment has been minimized.

getdave commented Nov 6, 2019

This comment has been minimized.

getdave commented Nov 6, 2019 • edited

This comment has been minimized.

retrofox commented Nov 6, 2019

This comment has been minimized.

retrofox commented Nov 6, 2019

This comment has been minimized.

retrofox commented Nov 6, 2019

This comment has been minimized.

getdave commented Nov 6, 2019

getdave commented Nov 5, 2019 •

edited

getdave commented Nov 6, 2019 •

edited