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

Auto-slug block and field names, make Repeater work #51

Merged
merged 57 commits into from
Dec 17, 2020

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Dec 3, 2020

Updated December 16th

Tickets

GF-2518: Auto-slug for block name
GF-2519: Auto-slug for field names
GF-2507: Settings for Repeater
GF-2508: Add / Remove Sub-fields
GF-2509: Move / Resize Sub-fields
GF-2510: Sub-field JSON data
GF-2511: Save sub-field data

Changes

  • Enables Repeater sub_fields
  • When there is no block name (slug), this sets it when entering the block title:
    there-slugging

Testing instructions

  1. Follow 'Testing Instructions' 1-5 in https://github.com/studiopress/genesis-custom-blocks-pro/pull/16#issue-515954659
  2. Test auto-slugging for the block name (GF-2518):
    adsf-asdsdf
  3. Expected: if you click somewhere else and edit the title again, it shouldn't auto-slug anymore
  4. Test auto-slugging for the field name (GF-2519):
    ijsdf-=sdf
  5. Expected: if you click somewhere else and edit the Field Label again, it shouldn't auto-slug anymore
  6. Change a field to 'Repeater'
  7. Expected: these 3 settings appear (GF-2507):
    isdfoij
  8. Test adding and deleting Repeater sub_fields (GF-2508): iojsdfsf
  9. Test moving and resizing Repeater sub_fields (GF-2509):
    in-drig
  10. Try to break any of the above 😄

Maybe revisit some of this if
there are bugs, but a lot of this
is for the block editor,
not for an editor that doesn't use blocks.
Sometimes, ref.current doesn't have
an ownerDocument property.
Then, it's not possible to get its properties.
Needed to import an NPM package.
Like how the previous editor worked,
so the user will enter the field name
right away.
@kienstra
Copy link
Contributor Author

kienstra commented Dec 3, 2020

Like in the previous editor, this focuses the field label <input> when adding a new field:

new-field-fo

On editing the field label for
a new field, auto-slug
the field name.
Even on blurring, allow autoslugging
the next time there's a change
There's no need for a rerender on changing this.
So don't call useState().
@kienstra kienstra changed the title Autoslug block name (slug) Autoslug block and field names Dec 4, 2020
getFieldIcon() gets
the icon, with a filter.
We might have to use <FieldsGrid>
in another component,
so move the parts that won't be needed in the
new one.
@kienstra kienstra changed the title Autoslug block and field names Autoslug block and field names, allow subfields Dec 7, 2020
@kienstra
Copy link
Contributor Author

kienstra commented Dec 7, 2020

The sub_fields now display, though there's a lot of work left with them:
the-sub-fields

This allows passing a parentName,
which will mark this as a sub_field.
So on clicking a sub_field,
it will set a name of the sub_field,
and a parentName of the parent field.
@kienstra
Copy link
Contributor Author

kienstra commented Dec 9, 2020

Here's the current editor:

creating-a-block

Add a warning on trying to exit with
unsaved changes.
There's still a lot of work
needed for sub_fields, though.
return;
}

// A hack to remove blocks from the edited entity.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The forked <EditorProvider> removes the need for this hack.

>
<button
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This diff is mainly from indentation.

return [];
}

return block.keywords.filter( ( keyword ) => Boolean( keyword ) );
Copy link
Contributor Author

@kienstra kienstra Dec 16, 2020

Choose a reason for hiding this comment

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

The previous editor sometimes saved block.keywords as [ '' ], which looked like:

keyword-hisd

So this .filter() removes '' keyword values.

* @param {LocationButtonProps} props
* @return {React.ReactElement} The location buttons.
*/
const LocationButtons = ( {
Copy link
Contributor Author

@kienstra kienstra Dec 16, 2020

Choose a reason for hiding this comment

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

Here's where this appears:
location-sd

It looks like this didn't
pass anything that was needed.
? control.settings.map( ( setting, index ) => {
if ( field.hasOwnProperty( 'parent' ) && 'location' === setting.name ) {
Copy link
Contributor Author

@kienstra kienstra Dec 16, 2020

Choose a reason for hiding this comment

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

This prevents changing the location of a sub_field, as it's inside a field that has a location:

the-sdf

@@ -189,7 +189,7 @@ protected function register_block( $block_name, $block ) {
$block_name = str_replace( '_', '-', $block_name );

// register_block_type doesn't allow slugs starting with a number.
if ( is_numeric( $block_name[0] ) ) {
if ( isset( $block_name[0] ) && is_numeric( $block_name[0] ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related, but found a PHP warning or error when testing.

@kienstra kienstra changed the title Autoslug block and field names, allow subfields Autoslug block and field names, make Repeater work Dec 16, 2020
A type should ideally
only be defined in one place,
and other files should import them.
} }
onBlur={ () => {
if ( didAutoSlug.current ) {
setIsNewField( false );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On blurring this <input>, it should prevent auto-slugging again.

</div>
{ null === parentField && 'repeater' === field.control
? (
<FieldsGrid
Copy link
Contributor Author

@kienstra kienstra Dec 16, 2020

Choose a reason for hiding this comment

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

A repeater also has a grid of fields. So it recursively renders this component:

a-isdf

@kienstra kienstra marked this pull request as ready for review December 16, 2020 09:11
@kienstra kienstra changed the title Autoslug block and field names, make Repeater work Auto-slug block and field names, make Repeater work Dec 16, 2020
Copy link

@mikemcalister mikemcalister left a comment

Choose a reason for hiding this comment

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

This is more of a UI and frontend test than a deep code dive, but everything was working well in my experiments.

  • Focus on title works with new block creation
  • Auto-slug on block and field name works
  • New settings (help, min/max rows) showing on repeater field
  • Adding and deleting repeater fields works as expected
  • Resizing repeater fields wasn't working initially, but there was no console error. I did a rebuild and they do work now. Seems to work well with no console errors.

Great work, @kienstra! I'm loving these improvements.

@kienstra
Copy link
Contributor Author

@mikemcalister,
Thanks so much for testing this and your details about how it went. I'll keep an eye out for resizing repeater fields, also.

@kienstra kienstra merged commit 9711172 into feature/react-ui Dec 17, 2020
@kienstra kienstra deleted the add/editor-ui-autoslugging branch December 17, 2020 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants