QuickEdit: Add slug field control #65196

gigitux · 2024-09-10T12:14:06Z

What?

Screen.Capture.on.2024-09-10.at.12-42-17.mp4

This PR adds the slug field control to the Page view. Most of the logic is copied from:

Currently, these two functions are public selectors and rely on the editor state, so we can't refactor those. I think that for now, we can keep this duplicate code and when the post editor sidebar will be powered by the dataview, we can think to refactor those, but I'm open to feedback on this.

Similar to #64496, this code could be moved to the @wordpress/core-fields-control package if we agree to create it.

Validation

There is an edge-case that I think that we should take care, but I will wait for your feedback before to proceed. When the input is empty, the save button is clickable:

This isn’t ideal because a page/post should always have a slug set. Currently, if the input is empty, the original slug is restored on the onBlur event, which is not the best UX.

How?

Testing Instructions

Ensure that Quick Edit in DataViews and Quick Edit in DataViews are enabled.

Visit the Site Editor.
Click on pages.
Toggle to the table view.
Click on the settings.
Click on details panel icon.
Ensure that the link field is visible
Click on it.
Update the slug.
Save.
Ensure that the page is visible to the new link.
Visit /wp-admin/options-permalink.php.
Select plain setting.
Save it.
Repeat 1-7 steps.
Ensure that you can't update the slug.

jameskoster · 2024-09-10T14:49:51Z

There is an edge-case that I think that we should take care, but I will wait for your feedback before to proceed. When the input is empty, the save button is clickable:

This is a tricky one, and technically an existing issue (see video below) so perhaps not one we have to fix here?

slug.mp4

Some validation might help; maybe the field should error when empty, and revert to the current value on blur? The Save button would be disabled when the field is empty. Some validation to avoid duplicate slugs might be handy as well.

We also need to think about the conditions in which the Link field is shown in the quick-edit panel. In the vast majority of cases it's not possible to apply the same slug to multiple items, so maybe we just hide it for now?

gigitux · 2024-09-10T15:01:00Z

This is a tricky one, and technically an existing issue (see video below) so perhaps not one we have to fix here?

Yeah, this is something that I noticed too.

Some validation might help; maybe the field should error when empty, and revert to the current value on blur? The Save button would be disabled when the field is empty.

I'm fine following this approach. I'm going to try to implement it!

Some validation to avoid duplicate slugs might be handy as well.

Yeah good point. I would see this kind of validation on the backend side when the user clicks save. For now, I would skip this point because it impacts some work not specifically related to the scope of this project.

We also need to think about the conditions in which the Link field is shown in the quick-edit panel. In the vast majority of cases it's not possible to apply the same slug to multiple items, so maybe we just hide it for now?

True! Let's hide it!

Thanks for your precious feedback, @jameskoster! 🙇

…into add/link-control

…control

oandregal · 2024-09-12T11:15:08Z

packages/edit-site/src/components/post-fields/index.js

@@ -369,6 +370,13 @@ function usePostFields( viewType ) {
 					return <time>{ getFormattedDate( item.date ) }</time>;
 				},
 			},
+			{


This works because our APIs are already extensible. However, there's an alternative that @joshuatf mentioned somewhere. Instead of doing:

{ id: 'slug', Edit: /* custom slug Edit */, render: /* custom slug render */, isValid: /* custom slug isValid */, // etc. }

we can do something along these lines:

registerFieldType( 'core/slug', FieldSlug ); const fields = [ { id: 'slug', type: 'core/slug', } ];

This has a few advantages:

It gets us a bit closer to making the type field required. We're not there there yet because there are a couple of missing types we need (image, for example), but we should aim to do it.

We offer higher semantics and gets us a bit closer to having no-code JSON-oriented fields. This may sound a bit abstract, but imagine the following scenario: in the future, there's a mechanism for users to create screens based on declaring which fields they want from a dataset. All via UI. In that scenario, that UI needs to be no-code, and users shouldn't have to provide a "custom render", "custom edit", "custom validation", etc. if they just want a field to be a "slug". Instead, they should work with types: the basic ones provided by DataViews (string, integer, date, etc.) but also any other one that is domain-specific (slug, cart-status, etc.).

Another aspect of this PR is what should be the API offered by the wordpress/field package?

This suggest it should export individual field properties (render, edit, isValid, etc.). There are a few. What if it exported the whole field instead? An advantage of exporting the whole Field is that the wordpress/field package can implement new APIs as they come without having to modify the consumers:

// The consumer doesn't have to modify any code when // a new Field property is introduced, renamed it, etc. // It's the FieldSlug responsibility to implement it. const fields = [ { id: 'slug', type: 'core/slug' } ];

…control

gigitux · 2024-09-18T09:48:41Z

We also need to think about the conditions in which the Link field is shown in the quick-edit panel. In the vast majority of cases it's not possible to apply the same slug to multiple items, so maybe we just hide it for now?

With dc2e1b8, the slug field is hidden in bulk edit mode.

After a chat with @louwie17 and @joshuatf we are inclined to merge this PR without the validation (given that currently there isn't in the post editor). I'm going to mark this PR as ready to be reviewed!

This is a tricky one, and technically an existing issue (see video below) so perhaps not one we have to fix here?

Regarding the validation, we are planning to use this field as playground to think/implement how the validation should look. We are planning to create a dedicated PR (or multiples) for the validation!

This has a few advantages:

It gets us a bit closer to making the type field required. We're not there there yet because there are a couple of missing types we need (image, for example), but we should aim to do it.

We offer higher semantics and gets us a bit closer to having no-code JSON-oriented fields. This may sound a bit abstract, but imagine the following scenario: in the future, there's a mechanism for users to create screens based on declaring which fields they want from a dataset. All via UI. In that scenario, that UI needs to be no-code, and users shouldn't have to provide a "custom render", "custom edit", "custom validation", etc. if they just want a field to be a "slug". Instead, they should work with types: the basic ones provided by DataViews (string, integer, date, etc.) but also any other one that is domain-specific (slug, cart-status, etc.)

Thanks for your comment! We are planning to work on this new API in the upcoming weeks!

gigitux · 2024-09-18T09:49:52Z

packages/fields/src/fields/slug/style.scss

@@ -0,0 +1,18 @@
+.fields-controls__slug {


Does it make sense this classname? Or should we think a better one?

Perhaps I'd go with .field-slug as a prefix.

oandregal · 2024-09-26T17:31:29Z

packages/fields/package.json

@@ -9,7 +9,6 @@
 		"gutenberg",
 		"dataviews"
 	],
-	"private": true,


I presume this needs to be rebased from trunk, correct? This package was made public already.

oandregal · 2024-09-26T17:34:38Z

packages/fields/src/fields/slug/index.ts

+import SlugView from './slug-view';
+
+const slugField: Field< BasePost > = {
+	id: 'slug',


What type would this field be? I feel we need to make progress on making type obligatory and every new field we introduce should have a type.

I would say slug, what do you think? Mostly, because this field contains specific logic to preview the final link based on the slug setting:

gutenberg/packages/fields/src/fields/slug/slug-edit.tsx

Lines 38 to 46 in 4d1f022

const permalinkPrefix = prefix;

const permalinkSuffix = suffix;

const isEditable = PERMALINK_POSTNAME_REGEX.test( permalinkTemplate );

const originalSlug = useRef( slug );

const slugToDisplay = slug || originalSlug.current;

const permalink = isEditable

? `${ permalinkPrefix }${ slugToDisplay }${ permalinkSuffix }`

: safeDecodeURIComponent( data.link || '' );

After further consideration, the type should always represent a generic type, not strictly tied to WordPress. In this case, it should be link. In the future, we might have:

A generic field for the link type.

Other fields that work with the link type but have additional functionalities that depend on specific settings like this one.

Would it make sense for each field to have a name?

Based on this discussion, I updated the type with 4efda74

oandregal · 2024-09-26T17:42:19Z

packages/edit-site/src/components/post-fields/index.js

@@ -369,6 +370,7 @@ function usePostFields( viewType ) {
 					return <time>{ getFormattedDate( item.date ) }</time>;
 				},
 			},
+			slugField,


It's ok for this PR going this way. I'd like to offer some thoughts about next steps.

Initially, the wordpress/fields package exported every specific part of a field (edit, label, etc.) and we changed it to the whole field. Now that I see this, I think we have to offer an even higher-level API:

const postFields = usePostFields(); const pageFields = usePageFields(); // etc.

Essentially, the same we do for actions. In the future, we'll want 3rd parties to be able to register/unregister fields, so we can't use individual fields in every screen — otherwise, every screen will have to provide a filter/registry for that. Instead, we should offer an API that gives all the fields that are registered for a given post type.

Initially, the wordpress/fields package exported every specific part of a field (edit, label, etc.) and we changed it to the whole field. Now that I see this, I think we have to offer an even higher-level API:

I see what you mean. I think that we will start to work on similar API when we will create a fields store in the @wordpress/fields package.

In the future, we'll want 3rd parties to be able to register/unregister fields, so we can't use individual fields in every screen — otherwise, every screen will have to provide a filter/registry for that

On this, we are full aligned! We will plan to work on this API.

I've prepared a few small PRs to move the remaining field definition to @wordpress/fields in preparation for moving usePostFields as well:

comment_status Post fields: move comment_status from edit-site to fields package #66934

status Post fields: move status from edit-site to fields #66937

date Post fields: move date fields from edit-site to fields package #66938

author Post fields: move author from edit-site to fields package #66939

title Post fields: extract title from edit-site to fields package #66940

…control

oandregal · 2024-10-23T09:18:28Z

packages/fields/README.md

@@ -66,6 +66,10 @@ Undocumented declaration.

 Undocumented declaration.

+### slugField


Can we start adding some JSDoc comments to the exported entities so this is filled up with actual info? It's enough to add a JSDoc to this statement.

oandregal · 2024-10-23T09:30:39Z

There is an edge-case that I think that we should take care, but I will wait for your feedback before to proceed. When the input is empty, the save button is clickable:

It's ok to address this in a follow-up, but wanted to share some thoughts:

we already validation in fields (isValid)
it's already built-in in the types (if your field declares a type it doesn't have to have its own isValid)
there's an utility to validate all fields based on their isValid callback: isItemValid that we are already using in some scenarios (reorder page modal)

Thinking about how to scope next steps, this is what I think we should do:

Connecting the existing isItemValid utility to the save button in the site editor.
Implement isValid in the slug field introduced here to check for non empty.

Note that when we introduce "slug" field type, that should be automatically created for the field — but that requires other things we don't have yet (being able to register new fields types).

oandregal · 2024-10-23T09:55:19Z

In terms of design, I've compared this to the one we have in the editor:

QuickEdit	Editor

I've noticed a couple of differences, we should probably align this PR with the editor.

Name: slug (quickedit) vs link (editor)
Missing text "Permalink:" before permalink

oandregal · 2024-10-23T10:50:56Z

I've noticed that the permalink setting doesn't affect the slug field. We either render it disabled (plain) or render the post name (any other option).

Not something to fix in this PR because it also happens in the editor, but I suppose we'd want to better expose that setting in the slug?

…control

oandregal

I think we can merge this one and continue iterating.

gigitux · 2024-10-23T15:54:42Z

In terms of design, I've compared this to the one we have in the editor:

Nice catch! The design has been updated in the last weeks: https://github.com/WordPress/gutenberg/pull/63669/files. I updated the design!

Co-authored-by: gigitux <[email protected]> Co-authored-by: oandregal <[email protected]> Co-authored-by: jameskoster <[email protected]>

Page view: Add link field control

5682b92

gigitux changed the title ~~Page view: Add slug field control~~ Dataview - Page view: Add slug field control Sep 10, 2024

gigitux changed the title ~~Dataview - Page view: Add slug field control~~ Data View - Page view: Add slug field control Sep 10, 2024

youknowriad requested review from oandregal and jameskoster September 10, 2024 13:47

gigitux added 10 commits September 11, 2024 10:05

Add @wordpress/fields package

395cc25

Merge branch 'add/@wordpress-fields' of github.com:gigitux/gutenberg …

e820a6c

…into add/link-control

remove not used references

b00ec03

add marker to generate API documentation

6e71a98

improve package description

43b8a93

not use lock/unlock api

7e04069

remove apostrophe

e5fd361

fix build

9bfd08f

Merge branch 'add/@wordpress-fields' of github.com:gigitux/gutenberg …

30def0b

…into add/link-control

move slug field control to @wordpress/fields package

0855668

gigitux mentioned this pull request Sep 11, 2024

Add @wordpress/fields package #65230

Merged

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/link-…

bc60075

…control

oandregal reviewed Sep 12, 2024

View reviewed changes

oandregal mentioned this pull request Sep 12, 2024

QuickEdit: Add Featured Image Control #64496

Merged

gigitux added 4 commits September 17, 2024 10:29

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/link-…

d95c35d

…control

improve definition

934bc1e

disable slug field control in bulk mode

dc2e1b8

improve css classnames

500b848

gigitux commented Sep 18, 2024

View reviewed changes

gigitux marked this pull request as ready for review September 18, 2024 09:50

gigitux requested review from ajitbohra and juanmaguitar as code owners September 18, 2024 09:50

oandregal changed the title ~~Data View - Page view: Add slug field control~~ QuickEdit: Add slug field control Sep 26, 2024

oandregal reviewed Sep 26, 2024

View reviewed changes

gigitux added 3 commits October 1, 2024 15:12

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/link-…

4d1f022

…control

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/link-…

d7e0e6f

…control

add type property

4efda74

gigitux requested a review from oandregal October 22, 2024 15:12

oandregal assigned gigitux Oct 23, 2024

oandregal added [Type] Experimental Experimental feature or API. [Package] DataViews /packages/dataviews labels Oct 23, 2024

oandregal reviewed Oct 23, 2024

View reviewed changes

gigitux added 2 commits October 23, 2024 16:20

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/link-…

f53239c

…control

update layout

075a2b0

oandregal approved these changes Oct 23, 2024

View reviewed changes

oandregal merged commit 1ca8001 into WordPress:trunk Oct 23, 2024
63 checks passed

github-actions bot added this to the Gutenberg 19.6 milestone Oct 23, 2024

gigitux mentioned this pull request Oct 24, 2024

Data Views - Slug Field Control: Implement validation #66397

Open

gigitux mentioned this pull request Sep 26, 2024

DataForm component #59745

Open

21 tasks

psealock mentioned this pull request Nov 22, 2024

[Settings] Introduce DataForms to Settings woocommerce/woocommerce#52316

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QuickEdit: Add slug field control #65196

QuickEdit: Add slug field control #65196

gigitux commented Sep 10, 2024 •

edited

Loading

jameskoster commented Sep 10, 2024

gigitux commented Sep 10, 2024

oandregal Sep 12, 2024

gigitux commented Sep 18, 2024 •

edited

Loading

gigitux Sep 18, 2024

oandregal Oct 23, 2024

oandregal Sep 26, 2024

oandregal Sep 26, 2024

gigitux Oct 1, 2024

gigitux Oct 1, 2024

gigitux Oct 22, 2024

oandregal Sep 26, 2024 •

edited

Loading

gigitux Oct 1, 2024

oandregal Nov 12, 2024

oandregal Oct 23, 2024

oandregal commented Oct 23, 2024

oandregal commented Oct 23, 2024

oandregal commented Oct 23, 2024

oandregal left a comment

gigitux commented Oct 23, 2024

	const permalinkPrefix = prefix;
	const permalinkSuffix = suffix;
	const isEditable = PERMALINK_POSTNAME_REGEX.test( permalinkTemplate );
	const originalSlug = useRef( slug );
	const slugToDisplay = slug \|\| originalSlug.current;
	const permalink = isEditable
	? `${ permalinkPrefix }${ slugToDisplay }${ permalinkSuffix }`
	: safeDecodeURIComponent( data.link \|\| '' );

		@@ -66,6 +66,10 @@ Undocumented declaration.

		Undocumented declaration.

		### slugField

QuickEdit: Add slug field control #65196

QuickEdit: Add slug field control #65196

Conversation

gigitux commented Sep 10, 2024 • edited Loading

What?

Validation

How?

Testing Instructions

jameskoster commented Sep 10, 2024

gigitux commented Sep 10, 2024

Choose a reason for hiding this comment

gigitux commented Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Oct 23, 2024

oandregal commented Oct 23, 2024

oandregal commented Oct 23, 2024

oandregal left a comment

Choose a reason for hiding this comment

gigitux commented Oct 23, 2024

gigitux commented Sep 10, 2024 •

edited

Loading

gigitux commented Sep 18, 2024 •

edited

Loading

oandregal Sep 26, 2024 •

edited

Loading