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

Two checkbox controls in PluginPostStatusInfo – clicking one triggers both #12289

Closed
florianbrinkmann opened this issue Nov 25, 2018 · 4 comments · Fixed by #12331
Closed
Assignees
Labels
[Type] Bug An existing feature does not function as intended

Comments

@florianbrinkmann
Copy link

Describe the bug

I have two plugin installed, that both add a checkbox to the publish area via PluginPostStatusInfo. If one of both is already clicked and I reload the editor, both get triggered if I click the box that is not checked. If both are checked, both get unchecked. After the first click, it works.

But the changed status of the not-clicked checkbox does not get saved – after clicking update, the correct status gets highlighted again.

To Reproduce

Steps to reproduce the behavior:

Not so easy, the checkbox values are depending on custom meta values.

I created a Gist with the code of both checkboxes in one file, so you can take a look how the code is: https://gist.github.com/florianbrinkmann/2c1fd1b7477f28354d65380dcc8c4170

Expected behavior
Only the one checkbox should be triggered.

Screenshots

two-checkbox-issue

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Firefox
  • Version: 65.0a1 (2018-11-24) (64-Bit)

Additional context

  • Gutenberg 4.5.1.
@swissspidy
Copy link
Member

According to the comment on #10827, that PR might be related to this behavior.

In a plugin of mine that hooks into PluginPostStatusInfo I also use editPost in the onChange callback, but I merge old and new meta data there so this does not happen.

See https://github.com/swissspidy/video-post-type/blob/0942bc7d6dc08ac1f5841032dbed295cd74a6670/assets/js/src/components/video-panel/index.js#L43-L63 for an example.

Could you try that in your plugin as well and let us know if it works? :-)

@youknowriad
Copy link
Contributor

I merge old and new meta data there so this does not happen.

Yes, this should solve the issue and this is how we were supposed to do it before #10827 anyway. So It's not really a regression but with #10827, in theory we should be able to apply partial changes, which means if we update the getEditedPostAttribute selector to "merge" automatically edited metas with non-edited ones, the code here will just work.

@aduth aduth self-assigned this Nov 26, 2018
@aduth aduth added the [Type] Bug An existing feature does not function as intended label Nov 26, 2018
@florianbrinkmann
Copy link
Author

florianbrinkmann commented Nov 26, 2018

Thanks @swissspidy, that works! Honestly, I don’t exactly understand why, but will try to dive into that topic later :)

@aduth
Copy link
Member

aduth commented Nov 26, 2018

While @swissspidy's workaround may be effective, I'd still consider this to be a bug in how we fail to merge meta properties with edited values from the getEditedPostAttribute selector. A developer shouldn't need to pass all meta properties through to editPost if they're not in-fact intending to edit all meta properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants