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

Allow isUnmodifiedBlock to bypass certain (unique/autogenerated attributes) #48806

Open
albanyacademy opened this issue Mar 6, 2023 · 7 comments
Labels
[Feature] Block API API that allows to express the block paradigm. Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.

Comments

@albanyacademy
Copy link

albanyacademy commented Mar 6, 2023

What problem does this address?

In a nutshell, when using filters [blocks/registerBlockType] and/or [editor.BlockEdit] to automatically generate unique block ids, this indicates to the editor that these blocks are now modified.

I know theres several issues about handling unique attributes within Gutenberg, but this deals with auto-generated attributes specifically.

In my opinion, automatically generated attribute values should be optionally ignored when considering if the block has been modified - what we really care about is user input that modifies the block, not something like an automatically generated block ID. we don't care about knowing whether or not the block ID is generated.

Some of this is partially addressed via __unstableMarkNextChangeAsNotPersistent from a UI perspective, however there's a lot of builtin functionality within the editor that utilizes isUnmodifiedBlock specifically, particularly the block inserter.

On a fresh paragraph block, an inserter appears that allows the user to replace that block with another, as core/paragraph functions as a placeholder.

When adding an autogenerated attribute to core/paragraph - or using it in a custom block that then uses setDefaultBlockName to override core/paragraph - this behaviour stops, and the block has been treated as modified despite the user only just clicking add new post and waiting for the page to load.

What is your proposed solution?

I propose adding a flag to the attribute itself that will tell isUnmodifiedBlock to ignore it.

https://github.com/WordPress/gutenberg/blob/trunk/packages/blocks/src/api/utils.js#L40

function isUnmodifiedBlock( block ) {
    // Cache a created default block if no cache exists or the default block
    // name changed.
    if ( ! isUnmodifiedBlock[ block.name ] ) {
        isUnmodifiedBlock[ block.name ] = createBlock( block.name );
    }

    const newBlock = isUnmodifiedBlock[ block.name ];
    const blockType = getBlockType( block.name );


    return Object.keys( blockType?.attributes ?? {} ).every(
        ( key ) => {
                const attr = blockType.attributes[key];
                if (attr?.ignoreModified === true) {
                    return true;
                }
                return  newBlock.attributes[ key ] === block.attributes[ key ];
        }
    );
}

ignoreModified kinda sucks as a name, but this is the gist of it.

@albanyacademy
Copy link
Author

Or opening up isModifiedBlock to hooks so you can apply your own custom logic instead, either or.

@albanyacademy
Copy link
Author

Something I just thought about - does isUnmodifiedBlock account for innerblocks? seems like it wouldn't check that.

@albanyacademy
Copy link
Author

piggybacking off of #46079 and #29693, this could be added as an additional __experimentalRole value, though still not sure on exact terminology.

@kathrynwp kathrynwp added [Type] Enhancement A suggestion for improvement. [Feature] Block API API that allows to express the block paradigm. Needs Technical Feedback Needs testing from a developer perspective. labels Mar 7, 2023
@talldan
Copy link
Contributor

talldan commented Apr 11, 2023

@albanyacademy What are the ids you're generating for? HTML ids?

@noisysocks has been exploring some adjacent topics:

But those might not cover your use case.

@gziolo
Copy link
Member

gziolo commented Apr 18, 2023

I think it's also related to the following issue #21703 where there is a discussion on how to relax validation to account for the cases like the one raised in this issue.

So there needs to be a mix of changes that allow adding additional HTML attributes that don't invalidate the block when the user doesn't provide them in UI. In effect, we want to be in a position where the validation is only concerned about the parts supplied by the user but not for the additional attributes not defined in the save implementation.

@noisysocks
Copy link
Member

Just so that I better understand the context, could you please describe your use case for the autogenerated attribute?

This speaks to me in favour of having #49391 i.e. a new abstraction that stores data alongside a block but isn't a block attribute.

@albanyacademy
Copy link
Author

Just so that I better understand the context, could you please describe your use case for the autogenerated attribute?

This speaks to me in favour of having #49391 i.e. a new abstraction that stores data alongside a block but isn't a block attribute.

sorry for the late reply lol

this was moreso for abstract stuff, not html IDs (though it could easily be used as a way to autopopulate them as well)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants