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

Drag/Drop causing errors while autosave is in-progress #823

Closed
arne-vdw opened this issue Dec 21, 2023 · 20 comments
Closed

Drag/Drop causing errors while autosave is in-progress #823

arne-vdw opened this issue Dec 21, 2023 · 20 comments

Comments

@arne-vdw
Copy link

Bug Description

Data from fields is removed when dragging-and-dropping while a draft autosave is in-progress.

drag-drop-bug.mp4

Steps to reproduce

  1. Add at least two Neo fields to a block.
  2. Make a change to one of the fields.
  3. While the entry is still auto-saving the draft, reorder the fields via drag-and-drop.
  4. When the problem occurs with a block you will see the fields of another block within the current block for a moment

Expected behaviour

Data should not be lost and the fields should be correctly reordered.

Neo version

3.9.11

Craft CMS version

4.5.12

What is the affected Neo field's propagation method?

No response

Does this issue involve templating, and if so, is eager-loading used?

This is not a templating issue

@ttempleton
Copy link
Contributor

I'm unable to reproduce this on the latest version (4.0.1) - but the root of these issues is due to us having to update blocks' visible layout elements after the draft autosave response (which includes the visible layout element data for the draft entry itself) which is due to Craft's draft autosave response currently not supporting including this data for nested/block fields such as Neo or Matrix. So there's a pretty good chance I can't reproduce this because I'm just getting the timing wrong during that process.

I'll continue to investigate this and update here if I can get it to happen. I'm guessing that this would be able to be resolved as of the initial Craft 5 release of Neo at the latest, though - since Matrix fields in Craft 5 will use entry types and therefore be allowed to have conditions, they will need to include the visible layout element data for those in the draft autosave response (relevant issue: craftcms/cms#13772), and would presumably let plugins include their own data as well.

@ttempleton ttempleton added the bug report status: confirmed A bug report that has been reproduced by the maintainer(s), but has not yet been fixed label Mar 19, 2024
@svale
Copy link

svale commented Mar 20, 2024

Hi,
we are seeing the same issue on a site recently upgraded to Craft 4. The bug seems pretty consistent in both production and dev environments. As a temporary workaround, we have asked the client to avoid drag'n drop if possible, but would very much like to present a better solution. Please let us know if we can help in any way to debug or test.

Versions:
Craft Pro 4.8.2
Neo 4.0.7

@ttempleton ttempleton added bug report status: unconfirmed A bug report that has not yet been reproduced by the maintainer(s), and more information is required and removed bug report status: confirmed A bug report that has been reproduced by the maintainer(s), but has not yet been fixed labels Mar 29, 2024
@svale
Copy link

svale commented Apr 10, 2024

I'd like to follow up on this as it is an ongoing issue for our client. We are seeing that content is lost on drag and drop.

Here is a screen recording of a simple re-production of just two blocks where all the content is dropped when re-arranged .

Screen.Recording.2024-04-10.at.11.55.01.mov

Please let me know if we can share further details to help investigate.

Current versions
Craft Pro 4.8.8
Neo 4.1.2

@ttempleton
Copy link
Contributor

@svale I continue to be unable to reproduce this. If possible, could you please send your composer files and database backup to [email protected] along with information about an entry where this is happening, and we'll have a look.

@svale
Copy link

svale commented Apr 12, 2024

Thank you for following up! Files and db are sent to [email protected].

@ttempleton ttempleton added bug report status: confirmed A bug report that has been reproduced by the maintainer(s), but has not yet been fixed and removed bug report status: unconfirmed A bug report that has not yet been reproduced by the maintainer(s), and more information is required labels Apr 14, 2024
@ttempleton
Copy link
Contributor

Thanks for sending those files @svale. I've made changes in the 4.x branch that I think have fixed this (or I'm having more trouble getting the timing right for it to happen).

Could you please run composer require spicyweb/craft-neo:"4.x-dev as 4.1.2" and let me know if you or your client experience this issue again?

@svale
Copy link

svale commented Apr 15, 2024

Hi @ttempleton , thank you for the update! The change is a clear improvement and it is now mostly no issue when moving blocks.

Unfortunately I can still trigger the error by re-ordering content - especially when testing with an image block which at a random point "copies" the content from a text block:

Before After
image image

(See the block labeled "Bilder")

While the issue was pretty constant before updating, it is much more random now, so absolutely on the right track.

@myleshyson
Copy link
Contributor

myleshyson commented May 8, 2024

Ok we just noticed this happening as well. I'm having a hard time reproducing it when just using a rich text fields, but I can produce it consistently when I have a rich text field and some other type of field. You can see the video of what's happening below.

A couple things I'm noticing.

  1. The autosave request fires while still holding a block.
  2. In the cases where this bug occurs, there's an autosave with the correct data, then one immediately after with blank data.
  3. It's not consistent when it happens, but if I drag certain blocks together it is consistent in happening at some point.

I wonder if pausing autosave while in a drag state, and triggering it after dropping would be the move here? There's some weirdness happening during drag, and i'm not sure if it's helpful to actually autosave before the user drops the block.

Pretty sure something like this will work. When I do this manually (just in chrome dev tools), I don't experience the issue anymore.

let editor = $('#main-form').data('elementEditor')

// on drag start
editor.pause()

// when dropped
editor.resume()

// force autosave
editor.checkForm(true)
screen.recording.-.1080WebShareName.mov

Craft: 4.9.0
Neo: 4.1.2
PHP: 8.2.13

@ttempleton
Copy link
Contributor

Do the changes in the 4.x branch resolve the issue for you?

@myleshyson
Copy link
Contributor

Oops. Just deleted a comment saying I couldn't reproduce it, but I actually can. It's a little harder now but it still happens. There's a little UI jump that occurs when the data clears (you can see the jump I'm talking about in the video)

@myleshyson
Copy link
Contributor

myleshyson commented May 9, 2024

I was trying to open a pull request but I'm having trouble getting the local env running. I'm using Orbstack instead of Docker for Mac so I think there's maybe some weirdness there.

Anyway, the change I was gonna suggest is this. Working for me locally. It also seems to fix the UI jump I was talking about. If the sort order changes, Craft will detect that and autosave the draft after drag end. If not, then no autosave will occur.

// Input.js
this._blockSort = new BlockSort({
  // ...A bunch of stuff
  onDragStart: () => {
    this.$form.data('elementEditor')?.pause()
  },
  onDragStop: () => {
    this.$form.data('elementEditor')?.resume()
    this._updateBlockOrder()
    this._updateButtons()
  }
})

@ttempleton
Copy link
Contributor

I've just made those changes in the 4.x branch, as well as some other changes to make sure the updating of visible layout elements in Neo blocks still happens.

@myleshyson @svale could you please confirm whether everything now works as expected with the current state of the 4.x branch?

@myleshyson
Copy link
Contributor

Ok yea that fixes it for me. Tried for a couple minutes to reproduce and could not reproduce the losing data bug. There is still this weird jump that happens when dropping a block in a new position, but it's a minor UI bug.

Screen.Recording.2024-05-10.at.9.06.31.AM.mov

@ttempleton
Copy link
Contributor

Thanks for that feedback @myleshyson.

I've just made some other changes to how and when visible layout elements are updated, to fix a separate bug (condition rules for parent block field values not always being applied), but I think it could have also fixed that bug in your video. Could you please run those latest changes in the 4.x branch and let me know if that's fixed it?

@svale
Copy link

svale commented May 14, 2024

Thank you for digging into this, @ttempleton . I'm also seeing an improvement in stability, but unfortunately I can still reproduce:

Screen.Recording.2024-05-14.at.12.13.23.mov

See text and image is lost.

(In the screen recording console log, ignore "'setTimeout' handler took ms", but I'm not sure if the redactor.js event listener warning could be relevant ? Probably not as I can also reproduce without any text-blocks )

Neo: 4.x-dev (b4aa37d)
Craft: 4.9.2

@ttempleton
Copy link
Contributor

Thanks @svale. Do you notice any difference in stability when swapping two previously existing blocks, compared with one or both of the blocks being new?

@harry2909
Copy link

harry2909 commented May 23, 2024

Hey just to add to the ongoing findings, we're also getting the above with content missing when moving blocks, however our client has also reported blocks disappearing entirely when saving as a new entry. It displays as a closed block on the entry, as if no block has been added.

Their steps to reproduce were:

  • Move a block around after cloning until content disappears.
  • Save the entry as a new entry.
  • Edit the new entry.
  • Visit the original entry -> blocks have totally disappeared, date modified does not reflect the actual date modified (its older)

Appreciate this may be a craft bug, but just wanted to note it down to see if anyone else has had similar issues.

Thanks!

Craft ver 4.9.2
Neo ver 4.1.2

@ttempleton ttempleton added this to the 4.2.0 milestone May 27, 2024
ttempleton added a commit that referenced this issue May 29, 2024
Backported from 5.x, hopefully will help with #823
ttempleton added a commit that referenced this issue May 29, 2024
@ttempleton
Copy link
Contributor

I suspect that some of the ongoing issues involve newly-created and unsaved blocks, so I've backported some changes from the 5.x branch to always load new blocks from the server, that have already been saved. I'm going to release this soon as version 4.2.0, regardless of whether it fixes this issue or not, but I'm hoping that it does fix this issue.

@ttempleton
Copy link
Contributor

I've just released 4.2.0. I'm going to close this issue in hope that it has been fixed, but please let me know if the issues persist for you after upgrading.

@ttempleton ttempleton added bug report status: fixed and removed bug report status: confirmed A bug report that has been reproduced by the maintainer(s), but has not yet been fixed labels May 30, 2024
@joepagan
Copy link

joepagan commented Jun 3, 2024

Hi @ttempleton just got the issue on release Neo release v4.20 which @harry2909 referenced here: #823 (comment)

When saving as a new entry, which has draft revisions, the original entry loses it's content.

Potentially happens after a specific job in the queue has completed so doesn't appear to happen immediately. Could be after the queue jobs: Deleting old Neo blocks or Saving Neo block structures for duplicated elements.

Manage to reproduce and capture the incident on a screen recording:

craftcms-neo-v4.20-content-disappearing-issue.mp4

Please can you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants