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

Replace FSE block save buttons with temporary fake setAttribute call #34350

Merged
merged 1 commit into from
Jun 28, 2019

Conversation

glendaviesnz
Copy link
Contributor

Changes proposed in this Pull Request

  • Remove the Site Description and Site Title FSE block save buttons and add a temporary setAttributes call to flag dirty state to editor to enable Update button to save changes

Testing instructions

  • Load this branch in your FSE testing environment.
  • Add a Site Description and Site Title block to a post
  • Publish/Update the post
  • Edit the Site Description block - check that the Update button becomes active again, then save
  • Edit the Site Title block - check that the Update button becomes active again
    attributeUpdate

Fixes #34287

@glendaviesnz glendaviesnz added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 28, 2019
@glendaviesnz glendaviesnz requested a review from a team June 28, 2019 00:09
@glendaviesnz glendaviesnz requested review from a team as code owners June 28, 2019 00:09
@glendaviesnz glendaviesnz self-assigned this Jun 28, 2019
@matticbot
Copy link
Contributor

// The following is a temporary fix. Setting this fake attribute is used to flag
// the content as dirty to editor and enable Update/Publish button. This should be
// removed once updating of site options handled in core editor
setAttributes( { updated: Date.now() } );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is interesting that this works to flag the editor state as dirty, even though the fse blocks do not have any attributes set and so this doesn't actually update any attribute data in the block content.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok to use this hack while we confirm if the Gutenberg behavior here is deliberate or not (paAmJe-tF-p2 #comment-1329).

Copy link
Contributor

Choose a reason for hiding this comment

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

Wacky.

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@glendaviesnz glendaviesnz added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 28, 2019
Copy link
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

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

Nice! Much better UX now without the save buttons.

Everything worked as expected on my tests so :shipit:

// The following is a temporary fix. Setting this fake attribute is used to flag
// the content as dirty to editor and enable Update/Publish button. This should be
// removed once updating of site options handled in core editor
setAttributes( { updated: Date.now() } );
Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok to use this hack while we confirm if the Gutenberg behavior here is deliberate or not (paAmJe-tF-p2 #comment-1329).

@glendaviesnz glendaviesnz merged commit 35aecf5 into master Jun 28, 2019
@glendaviesnz glendaviesnz deleted the update/remove-fse-block-save-buttons branch June 28, 2019 04:01
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.

Full Site Editing: Replace save buttons with temporary attribute changes [1]
4 participants