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

Page Front-end Editor Has Bugs In Master Clean Install #609

Open
denisurn opened this issue May 31, 2018 · 23 comments
Open

Page Front-end Editor Has Bugs In Master Clean Install #609

denisurn opened this issue May 31, 2018 · 23 comments

Comments

@denisurn
Copy link

@denisurn denisurn commented May 31, 2018

Short description of the issue

In latest master clean install Page Frontend Editor module throws quite a few bugs and errors.

Expected behavior

  • logged in user opens modal to edit some fields on the front end
  • user saves his changes and pressing save button closes the modal
  • changes are seen on the page

Actual behavior

  • user opens the modal;
  • javascript console throws few errors (which could be the culprit);
  • ajax preloader does not disappear;
  • pressing save button does not close the modal (data-autoclose does not work);
  • field changes are saved, but are not visible until manual page refresh;

Actual behavior goes here.

Screenshots/Links that demonstrate the issue

Firefox console
[Persistent ajax preloader] (https://screenshots.firefox.com/xXPjZgsD7DX0Z4rC/clean.local)

Steps to reproduce the issue

  1. Do a clean install of the latest PW master;
  2. Enable/install Page Front End Editor module;
  3. Add tag to the home template <edit page="<?=$page->id?>" fields="title,body">test modal</edit>
  4. Try to open the modal and save fields. Check the browser console for Jquery errors.

Setup/Environment

XAMPP, MAMP, tested in latest Firefox and Safari.

  • ProcessWire version: 3.0.98
  • Jquery Core module version: 1.8.3
@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jun 1, 2018

I can confirm this on Chrome too. PW 3.0.102, AdminThemeDefault and AdminThemeUikit.

I got this error in Firefox:
.ui-dialog:visible:last .ui-dialog-content is not a valid selector.

From Chrome:
Uncaught TypeError: $ is not a function
at HTMLIFrameElement.eval (eval at (JqueryCore.js:2), :1:2478)
at HTMLIFrameElement.dispatch (JqueryCore.js:2)
at HTMLIFrameElement.u (JqueryCore.js:2)

I tried with jquery 1.11.1 by setting the devmode to false in JqueryCore.module, but then tooltip is undefined.

BTW, in /wire/modules/Page/PageFrontEdit/PageFrontEdit.css is source map present.

@ryancramerdesign

This comment has been minimized.

Copy link
Contributor

@ryancramerdesign ryancramerdesign commented Jun 1, 2018

I'm not able to duplicate the issue in either Chrome or Firefox. However, the error seems to indicate jQuery isn't available, so I'm thinking it might have more to do with the front-end environment. Which site profile are you seeing it in? Perhaps PageFrontEdit is detecting that jQuery is present, when it isn't.

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 1, 2018

I am using minimal site profile to reproduce.

Is it possible to see you "Network" output? Mine is here (after opening the modal)

browser network output

All responses are fine (contain actual javascript response). Is there a file missing then?

@markosiilak

This comment has been minimized.

Copy link

@markosiilak markosiilak commented Jun 1, 2018

Same result here with "Steps to reproduce the issue"
I also Installed latest master with multi language profile installer.
I got a same error: at HTMLIFrameElement.u (JqueryCore.js:2)

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jun 1, 2018

I'm testing on existing installation. This is my html:

<!doctype html>
<html>
	<head></head>
	<body>	
		<edit fields="title,integer_field">test modal</edit>	
	</body>
</html>

js files loaded:

http://localhost/wire/modules/Page/PageFrontEdit/PageFrontEditLoad.js?NoMinify=1
http://localhost/wire/modules/Jquery/JqueryCore/JqueryCore.js
http://localhost/wire/modules/Jquery/JqueryUI/JqueryUI.js
http://localhost/wire/modules/Page/PageFrontEdit/PageFrontEdit.js?nc=1527818150

On dblclick other files are loaded, then it fail on this line in modal.js:

var k = $("<textarea />").text(h.contents().find("title").text()).html();

Should it be like this?

var k = jQuery("<textarea />").text(h.contents().find("title").text()).html();

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 1, 2018

@ryancramerdesign I think that @matjazpotocnik solution could be the bugfix. Briefly tested, fixed the issue.

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 4, 2018

@ryancramerdesign also, is the value on the page supposed to be updated without refresh?

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 4, 2018

Also, this line var abort = typeof event.toElement != "undefined" && jQuery(event.toElement).hasClass('fa-times'); is always false in Firefox (FF does not have toElement).

Adding event.target to the mix did not help, as it keeps returning iframe, not the <button> or <i>.

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 4, 2018

jQuery(event.toElement).hasClass('fa-times') also does not always work as sometimes a user clicks on the outer
There should definitely be an improvement on how the modal is closed with "x"

My solution (jQuery(event.toElement).hasClass('fa-times') || jQuery(event.toElement).hasClass('ui-dialog-titlebar-close'))

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 4, 2018

Also, when a modal window is closed, it is hidden, rather than destroyed. And when the modal is re-opened, it creates new element, rather than makes the old one visible.

Therefore, opening a new modal results in two elements with the same ID, which could potentially have other issues.

I see this code
for(var n = 0; n <= pwModalWindows.length; n++) { var $iframe = pwModalWindows[n]; if($iframe == null) continue; if($iframe.dialog('isOpen')) continue; $iframe.dialog('destroy').remove(); pwModalWindows[n] = null; }
but pwModalWindows does not seem to be initialized.

Possible solution:

jQuery.each(jQuery('.pw-modal-window'), function() { $iframe = jQuery(this); if ($iframe.dialog('isOpen')) return; $iframe.dialog('destroy').remove(); });

@denisurn

This comment has been minimized.

Copy link
Author

@denisurn denisurn commented Jun 6, 2018

var abort = typeof event.originalEvent != "undefined" && (jQuery(event.originalEvent.target).hasClass('fa-times') || jQuery(event.originalEvent.target).hasClass('ui-dialog-titlebar-close'));

Here is my solution for 'abort' result 'false' in FF. Tested in FF, Chrome and Edge.

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jun 6, 2018

On my front end something is injecting this in the :

    <link rel='stylesheet' href='/wire/modules/Inputfield/InputfieldCKEditor/InputfieldCKEditor.css?v=160-1526022034'>
    <script src='/wire/modules/Inputfield/InputfieldCKEditor/ckeditor-4.8.0/ckeditor.js'></script>
    <script src='/wire/modules/Jquery/JqueryUI/JqueryUI.js?v=1526022034'></script>
    <script src='/wire/modules/Jquery/JqueryUI/modal.js?v=1528267710'></script>
    <script src='/wire/modules/Inputfield/InputfieldCKEditor/InputfieldCKEditor.js?v=160-1526022034'>

It looks like that could cause problems:

JqueryUI.js?v=1526022034:6 Uncaught ReferenceError: jQuery is not defined
    at JqueryUI.js?v=1526022034:6

modal.js?v=1528267710:442 Uncaught ReferenceError: jQuery is not defined
    at modal.js?v=1528267710:442

InputfieldCKEditor.js?v=160-1526022034:40 Uncaught ReferenceError: ProcessWire is not defined
    at ckeLoadPlugins (InputfieldCKEditor.js?v=160-1526022034:40)
    at InputfieldCKEditor.js?v=160-1526022034:45 

I'm using option A and this happen when I enable field that is ckeditor field. I guess there is jQuery conflict as jQuery is loaded again later on...

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jun 6, 2018

Hmm, JqueryCore.js is not loaded at all, but it should be loaded before any other scripts...

In /wire/core/admin.php JqueryCore.js (and JquerUI) are loaded:

// ensure core jQuery modules are loaded before others
$modules->get("JqueryCore"); 
$modules->get("JqueryUI"); 

but in PageFrontEdit.module JqueryCore is not loaded before other scripts and $inputfield->renderReady() call also doesn't load JqueryCore. Or is it just me having problems? I fixed it by loading JqueryCore in JqueryUI.module since JqueryUI won't work without JqueryCore. Another option would be loading JqueryCore in Inputfields.module since almost all inputfields need it.

Also, required assets are placed just before closing (in PageFrontEdit.module line 397) and this is just to late since other scripts in the head are executed and I get errors like this:

Uncaught ReferenceError: ProcessWire is not defined
    at ckeLoadPlugins (InputfieldCKEditor.js?v=160-1526022034:40)
    at InputfieldCKEditor.js?v=160-1526022034:45

I added required assets/scripts just after </title>.

Also in PageFrontEdit.js there are jQuery calls but those should be $.

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jun 8, 2018

I found out why I had so many troubles with PFE. I was using $config->styles and $config->scripts on frontend in my templates. PFE injects assets (js scripts) using $inputfield->renderReady() method and I echoed those scripts, but jQuery wasn't there yet. So I modified my template rendering and PFE is working. Of course, those bugs that denisurn mentioned are still there...

ryancramerdesign added a commit to processwire/processwire that referenced this issue Jun 12, 2018
…swire-issues#609. Plus, this commit also improves PageFrontEdit usage with AdminThemeUikit, and improves behavior of multi-language fields.
@ryancramerdesign

This comment has been minimized.

Copy link
Contributor

@ryancramerdesign ryancramerdesign commented Jun 12, 2018

Thanks @denisurn and @matjazpotocnik, nice work. I've pushed several updates to PageFrontEdit per the fixes mentioned above (and a couple others).

@schwaaab

This comment has been minimized.

Copy link

@schwaaab schwaaab commented Jun 29, 2018

It's working, but not with jQuery 3.3.1, which I used. With the jQuery version from the module, it works as expected.

I guess it's time to upgrade from jQuery 1.11.1 to a more recent version. This problem will only appear more and more.

@schwaaab

This comment has been minimized.

Copy link

@schwaaab schwaaab commented Jun 29, 2018

Hitting enter instead of clicking save while editing in the modal does not close the modal. The whole admin page gets loaded in the modal in that case.

I don't know if this was always the case, but it feels wrong.

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jul 2, 2018

@schwaaab, In my testing it's working with option A, eg. inline edit, but it doesn't work with modal. You must use jQuery migrate (jquery-migrate-3.0.1.min.js) or use older jquery. Ryan doesn't want to upgrade jquery (yet).

@ryancramerdesign it would be good to replace deprecated functions in modal.js (.onload() and .resize()) with .on() function.

I tried upgrading jqueryui to jquery-ui-1.12.1.min.js but there are several problems.

@schwaaab

This comment has been minimized.

Copy link

@schwaaab schwaaab commented Jul 2, 2018

@matjazpotocnik I'm using the option D with html edit attributes, that way the modal is loaded, not the inline editor. I'm using jQuery 1.11.1 now. I meanwhile encountered following problems:

  • hitting enter does not close the modal, but loads the backend page in the modal
  • When an error occurs, like saving a empty mandatory field, the error is not handled correctly
  • the new UIKit theme is not completely applied

Better something than nothing though, I really like the module overall.

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jul 2, 2018

@schwaaab I tried with option C (<edit fields="title,body">test modal</edit>) and D (<div edit="title,body">test modal</div>) using jquery-3.3.1.min.js and jquery-migrate-3.0.1.js in my frontend <head>tag and it's working without problems as far as opening the modal. I only use AdminThemeDefault so maybe you should specify what's wrong so Ryan can fix it? Enter key is not trigered so autoclose is false and the modal stays open. It can be fixed (I did it by using on submit event). Errors are displayed here, but not ideal, I agree. You can fix that by setting the html required field on fileld settings. You could also try FEEL and combine it with inline PFE.

@matjazpotocnik

This comment has been minimized.

Copy link

@matjazpotocnik matjazpotocnik commented Jul 4, 2018

I found out that modal.js is interfering with click events on <a> element within the pwModalDoubleClick() function. I have an image gallery using fancybox (I guess it could be any other image gallery that opens modal) and after the image is shown, jQuery(document).on('click', '.pw-modal-dblclick a', function() { kicks in and trigger another unwanted click on image...

@x02a

This comment has been minimized.

Copy link

@x02a x02a commented Oct 19, 2018

Same Problem(s) here, when using Bootstrap 4 with jQuery 3.2.1 :(

@thetuningspoon

This comment has been minimized.

Copy link

@thetuningspoon thetuningspoon commented Jan 10, 2019

I also have the modal issues in PW 3.0.110 when using jQuery 3+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.