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

[3.2] Ctrl-s save unreliable, beginning in previews #4439

Closed
narration-sd opened this issue Jun 24, 2019 · 11 comments
Closed

[3.2] Ctrl-s save unreliable, beginning in previews #4439

narration-sd opened this issue Jun 24, 2019 · 11 comments

Comments

@narration-sd
Copy link
Contributor

narration-sd commented Jun 24, 2019

Description

This looks likely related to #4397, but no Categories are involved, just simple text fields.

  • there are a number of aspects apparent, and after losing a full description in edit, you'll get the much abbreviated version. It all does look like some event/race/state involvement, hence the number of ways problems actually can show.
  • because of this, to be confident of seeing the problems, each time please start by fully refreshing the browser on the Entry Edit main page. As things change after exercise, and I've even seen the alerts and related problems disappear then...
  • the most evident issue: Ctrl-S from a preview puts up an alert/dialog, 'Leave site? Changes you made may not be saved.' . In fact, if you agree, changes will be saved, Updated in 3.2 parlance, and you'll end up as expected in the main Entry editor.
  • but - if you cancel, now previews no longer update on typing (no refresh occurs). And if you then Close Preview, you see your typing, but the Update Entry button isn't lit, and it won't light with further typing either.

This should be enough to locate what's going on, but be assured there are a number of related faults that can occur with various pathways exercised around this, especially after first fail.

To be clear, I'm testing this time with no browser plugins, and seeing nearly identical issues on both Firefox latest non-dev release, and Chrome, this admittedly Canary. At the moment server is in a Vagrant box on the same i7 laptop as browser, and I have seen before that overall process/thread interleaving can heighten anything to do with update or event micro-delays.

Re: prior issue report, I am from time to time seeing the r.shift console report, but not in any sync with the ctrl-S errors, which occur just fine and reliably as stated without r.shift presence.

The one other thing I could mention is that alert -- what it's actually saying ('may lose work??', 'leave site?) -- but also whether it should present from previews. I remember we discussed this, and that it appears was a work-around, I think to do with discovering if anything had really been edited. I would truly be very sympathetic to possible difficulties...thought does occur at moment if you couldn't possibly know that by whether or not you sent any preview updates from Garnish-land?

Additional info

PHP version 7.1.28
OS version Linux 4.15.0-47-generic
Database driver & version MySQL 5.7.25
Image driver & version GD 7.1.28
Craft edition & version Craft Pro 3.2.0-beta.3
Yii version 2.0.21
Twig version 2.11.3
Guzzle version 6.3.3
Imagine version 0.7-dev
Plugins
Asset Rev 6.0.2
CraftQL dev-master
Element API 2.5.4
Live Vue dev-revised-craft-drafts
Redactor 2.3.3.2

May as well have a pic - also showing 3.2/draft preview capability looking pretty ready for integration into release!

preview-ctrl-s

@brandonkelly
Copy link
Member

First worth explaining how those unload confirmations work: when the page loads, Craft stores the form’s initial serialized state internally. Then on window.beforeunload it will compare the new serialized state with the stored one, and if so, tell the browser to show a confirmation dialog.

Live Preview has always been tricky, because when you open it, Craft actually moves the custom fields’ form elements over to a separate form within the editor pane that slides in, and replacing them within the main form with dumb copies of the fields (so there’s no visual change to the main form as the Live Preview panes are sliding in). So as long as Live Preview is open, there hasn’t been a reliable way to know whether to show the unload confirmation or not, so Craft just always shows it to be safe.

That said, I did just think of a way to solve this for Craft 3.2, thanks to some changes in how Live Preview works for entries. So hopefully that will start working more reliably going forward.

but - if you cancel, now previews no longer update on typing (no refresh occurs). And if you then Close Preview, you see your typing, but the Update Entry button isn't lit, and it won't light with further typing either.

This I can’t explain, nor reproduce. Maybe you’re getting a JavaScript error that is preventing other JS code from working?

@narration-sd
Copy link
Contributor Author

narration-sd commented Jun 24, 2019

Yeah, well understood. I'm glad you're thinking up a new method. Vy happy to test it when you get there; just let me know the repo to load.

As far as js errors, not printing on console, but that doesn't necessarily mean anything, as you well know. We could be still suspicious of old friend Redactor, given it presumably does cause the r.shift stack trace that prints occasionally. So again, your crisper method may well banish ghosts.

Worth saying, as I think again whether I could be causing anything, that the observations for this report come from running the older deprecated Live Vue architecture, quite different from new one for the other report. In both cases, there's of course a SPA running in the preview window. This one is a Vue spa; first one was a React/Gatsby app. I kind of think that all these differences tend to indicate nicht shuldig, to use the German.

Closing thought is about the thing you don't see but I do -- not always, given the ghostly behaviours all around: that the CP Entry Editor quits noticing that it's showing typed content adds still, but not responding to events to trigger the Update(able) armature. This again sounds all on the CP side w/grave suspicion of Redactor, so your fresh less complicated design may be very healthful, suspect. Cheers, Brandon.

Clive

@narration-sd
Copy link
Contributor Author

p.s. auto-switching-on-3.2 draft previews is all integrated, just the (fourth version) doc/website to write. I had first to get my head together on how this thing would be presented...when you see it, think you'll appreciate...

@brandonkelly
Copy link
Member

Vy happy to test it when you get there; just let me know the repo to load.

To get the fix early, change your craftcms/cms requirement in composer.json to:

"require": {
  "craftcms/cms": "3.2.x-dev#ba4b1b051b9db324708e85306f35f849b7223706 as 3.2.0-beta.3",
  "...": "..."
}

Then run composer update.

@narration-sd
Copy link
Contributor Author

Ok, about to try this. Had to clear up a repo thing first,mistyped tag, but this gave me a chance to think and test a little more on present 3.2 before diverting to your patch.

It showed something interesting: that previews are not necessarily involved! I can generate the false 'Leave site?' dialert from the plain CP Edit Entry page, just by hitting ctrl-s.

And if I cancel or use Escape to do same, the edit page is again locked in the doesn't trigger the Update button state.

Doing a browser page reload will revert to working again, and as expected page data is missing the ignored typing, showing then as last preview-edited, properly tagged as being on the Draft N page.

This looks very solid, so a good test now coming on the patch. Noticed also new report from Mac apparently in this area, so maybe two birds from one stone...let's see.

@narration-sd
Copy link
Contributor Author

And, success...great.

This appears to really solve the problem, with only one small thing or so left.

  • no more false 'Leave site?' dialerts, either on main Entry editor or preview. Sigh of relief ;)
  • ctrl-s just immediately saves as expected, either from editor or preview. Normal page appears, and Draft N which was in use disappears from the list, with Current tag showing. All clean.
  • though it's early days, no r.shift console traces either. One may suspect what you do now is taming Redactor, that's the hope. I'll let you know if I see it again.

All in all, this immediately feels much improved, Brandon.

The one thing left, is that there actually are no warnings now about leaving a page without Save/Updating. You just go out.

  • I was noticing this before your patch, just didn't mention it, one of the ghosts referred to. So this may be either separate, or as suspect, just needs different implementation to match the patch.
  • with your patch, it may well be just something you haven't gotten around to completing yet, well understood
  • if you go back into the page that was edited, you're properly in the Draft for it, showing anything that preview-altered/added in that draft, but missing anything done from main editor prior to leaving. Sensible to what is visibly happening, then.
  • when revisiting this, it would be very nice if the dialert didn't say 'Leave site?', as it's not the site you're leaving.... As well, probably it should be more definite that you will lose...your edit changes of this content, rather than that ''maybe, you will lose something??

You'll notice I am carefully not suggesting just what it should say ;)

I don't stop being impressed...appreciating especially this was a dusty corner.

@narration-sd
Copy link
Contributor Author

narration-sd commented Jun 24, 2019

Oh, yes. I do see a few extra Draft Ns in the list, I suspect only from failed leaving-of-edits, before and from the last issue just above. I guess these would go away in the 30 days for unused pages? Better you than me on the full maintenance picture of drafts ;), but you have your reasons for liking this approach am sure...

@brandonkelly
Copy link
Member

The one thing left, is that there actually are no warnings now about leaving a page without Save/Updating. You just go out.

You will still get the unload confirmation if you try to leave the page before it is finished autosaving the change to the current draft. But once the autosave is complete, there’s no need to warn you about leaving the page anymore; all your work is saved to the draft.

when revisiting this, it would be very nice if the dialert didn't say 'Leave site?', as it's not the site you're leaving.... As well, probably it should be more definite that you will lose...your edit changes of this content, rather than that ''maybe, you will lose something??

I agree, but customizing the text is a nonstandard feature that is only supported by IE.

@narration-sd
Copy link
Contributor Author

I agree, but customizing the text is a nonstandard feature that is only supported by IE.

Ah, yes. I'll remember not to bring this up again, as think I have. More a words than 'puter guy I guess always

You will still get the unload confirmation if you try to leave the page before it is finished autosaving

Hmm. I'll check that again if didn't seem so, once I finish bringing up a fresh live-vue-gatsby vm to run with 3.2, two birds

@narration-sd
Copy link
Contributor Author

narration-sd commented Jun 24, 2019

Ok, back, with a test on your patched 3.2, standard Entry editor only, as somehow my upgraded Live Vue isn't yet triggering in. :trollface:

  • indeed, if I have added a little text to the entry, and long after the Update button has turned red, clicking on the breadcrumbs to go back to the entry's Structure listing, Cards in this case as you'll note in the attached screenshot -- just goes there. No dialert.

  • Clicking back into the entry edit, I am now in Current. My changes don't show.

  • If I check the list, I find two things to notice:

    • There's a most-recent Draft, which does contain that last added text.
    • But, there's a fresh Draft for each time I do this. So I'm already up to Draft 4. Instead of re-using the recent Draft, as other ops like preview sensibly do.
  • If I click Update on the visible latest Draft N, I do go to Current, and this N disappears from the list.

I could guess this is all as intended. But it's a bit tricky for a neophyte content editor such as myself. And especially, besides apparently losing my edit at first, I'm wondering about the long list of Drafts I could accumulate, and over time.

Esp. if they aren't auto-deleted. Is there a way to manage that, besides I think a Craft-console command?

Adding the screenshot as said I would, but probably not relevant now. Pic is from prior to clicking away; is also state once you find and rejoin the held draft:

maybe-ok-base

@narration-sd
Copy link
Contributor Author

narration-sd commented Jun 24, 2019

Second thought on the doesn't-return-to-my-draft-rather-live-copy specific item is that maybe it's safer that way? That a content editor should just learn how it works?

There could be n more thoughts, seems clear. This feels complicated. And what happens if there are multiple editors trying same page.

I did put a cover on that, in the original Live Vue architecture, for the millisecond/s where they could possibly collide. Don't have do it now that your drafts are managing the show.

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

No branches or pull requests

2 participants