Refactors Gutenberg Feature #870
Conversation
There was a problem hiding this comment.
Thanks for your contribution!
I'm unable to test it, when I checkout this branch I get an error on the console when trying to interact with the sidebar element:
Uncaught TypeError: l.useSelect(...) is not iterable
Can you verify if everything is in place or maybe update the PR with some test instructions if any additional step is needed?
Thanks!
|
@leogermani are you able to give me a little more information about where that error occurs in the code or if the console gives you any additional context? I did make one update in which I could see an error being thrown, but I double-checked my code and I was not able to replicate that error. Do you have coauthors (guest authors) on your WP install for testing, or is it an empty author set? Perhaps if my fix does not solve that problem and that is the case I can re-check with zero coauthors. |
Tested in 2 different sites. One with Guest Authors and one without... I'm running Maybe some missing build step? |
|
@leogermani thanks for the info. I was able to replicate, resolve the failure, and retest with a fresh install. Please let me know if you continue to have the issue. |
|
To help with performance could we consider only performing the fetch when there's > 2 characters entered in the search field? A fetch is also performed on every key press, a delay of ~500ms would help reduce the number of unnecessary network calls. |
|
@elliott-stocks that's probably a good idea. I've made an update that allows this with a threshold of 2 added with an optional filter for modification. |
There was a problem hiding this comment.
It tests well for me, I only fixed the threshold check in f4cfa7a
On testing the feature, I noticed a few glitches and things we could improve, but nothing that was added by this PR, so let's not block it by these improvements.
In particular, we could have a debounce so we don't trigger multiple request while typing a user name
|
@leogermani @jomurgel As I was testing this I noticed that any changes to authors are reflected instantly, prior to saving the post. Is this a bug? I wouldn't expect any edits in the editor to take effect until I've saved the post. Previously the updates were handled within Happy to provide additional info if needed. |
|
I confirm there was a change in this behavior. But I also confirmed there was a bug in the previous implementation: Even though things were supposed to only be saved when you saved the post, the authors update was also triggered by an auto-save (in theory, auto saves are not applied to the post, it's saved separately) Considering this, maybe this is not a regression, but making it more explicit on how this works. What do you think @rebeccahum ? If in doubt, we can always revert it. |
|
@leogermani This feels like a regression to me, as any changes shouldn't be saved until the post has been saved. Is it possible to patch it to use |
Agree, but note that this already happens in a much more non explicit way, in the form of a bug. If you want to confirm this:
It's definitely possible, but on a quick attempt I encountered some other issues, so this would require more time I'm afraid. (until this day, there is not a straight forward way to trigger a callback when a post is saved - WordPress/gutenberg#17632 ) Maybe it's worth reverting this and taking some more time. |
|
I think it's worth reverting and sleeping on since I would prefer not to introduce a (further) regression. |
@leogermani We recently implemented a fix for this on our fork - eupolitico@230b1f3. That's based on the same PR, so it should work in this case - but let me know if not. (Thanks to to @Ritesh-patel). |
|
@elliott-stocks Did you want to make a PR for that fix? |
|
This implements the above patch. Works well for me |
Closes: #868
withState,withDispatch,withSelect.@wordpress/scriptspackage and updates thepackage.jsonscript for formatting.NOTE: The giant diff is a result of the
package-lock.jsonfile.