-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add missing method to list block. #529
Conversation
This looks like a good change, but I'm still seeing that this method is not being triggered and any updates are being lost. I'm also unable to focus the list block if I click within a |
Yeah good point, it is updating content but the actual items state is not being updated, should the list block keep track of both or should it be unified. Having two sources of truth kinda makes problems like this easy to miss good catch. In short:
|
Just realized there should be no content attribute for the list block oops. Will try and update properly. |
The list block does not have an on change set for it and is expected to causing errors and no updating of the block attribute state. This PR copies the current implementation from a text block and enters it into the list block.
72a9a0d
to
4bf2666
Compare
Okay rebased and updated, so the redux state for a list block will update properly on change. |
I'm still seeing an error on the console (and the state don't update properly) when I have a single What bothers me a bit is that the block author needs to be aware of the shape of the underlying data. I'm wondering if it simpler if we change the parsing of |
I didn't test for a single list item, so I will fix that. Does the data not come as an array? If not it should be like |
The data received from |
Ah then React made a bad choice then lol. |
Yeah.... this is super buggy lol. |
TinyMCE or something is doing some very bizarre things in the list block. The latest update seems to work a bit better as far as the state goes, but sometimes odd things appear in the state from TinyMCE. |
Adds in conditional logic to account for when there is a list of elements, a single element, or a null element.
@EphoxJames you might want to look at this |
I don't know enough about TinyMCE to know what it is doing. Basically sometimes the content will be an empty |
Just tested and the |
I just opened #567 which looks potentially related. However, the changes in this branch do not fix the issues I noted there - in fact, the new |
Anything that needs to be changed on this? |
@BE-Webdesign this PR didn't fix the issues noted in the description for me. They've since been fixed, though I'm not sure exactly where. Separately, #598 achieved most of what we wanted to do here, so I think we can close this one out. Please reopen if I missed something. |
The list block does not have an on change set for it and is expected to;
causing errors and no updating of the block attribute state. This PR
copies the current implementation from a text block and enters it into
the list block.