Fix #3279 warning closing form #3303

stu01509 · 2020-04-01T12:18:17Z

Short description of what this resolves:

Added warning dialog to show when the user closes the tabs.

stu01509 · 2020-04-01T12:19:42Z

Hello @bradymiller @stephenwaite @sjpadgett @tywrenn

Please review this PR, and give me feedback or suggestion about this dialog style.
Thanks :)

sjpadgett · 2020-04-01T12:47:29Z

Hi Cliff, while the workflow is correct for the alert, we don't want user having to do this for every tab closure. You think providers are stressed now, wait til we give them this:)

These type alerts should happen in forms where if content changed to trigger alert occurs.

stu01509 · 2020-04-01T12:52:59Z

Hi @sjpadgett

Thanks for your reply, If have any request changed Please let me know.

sjpadgett · 2020-04-01T13:13:18Z

Well if I understand @stephenwaite original issue, he was concern that a modified SOAP could be canceled w/o a second chance for user to reconsider. This is common in UX and should somehow extend to all our encounter forms.

I see two places to trigger the alert.

x-ed out from tab while form is being edited.
canceled in form while editing.

My first thought is a javascript function, maybe even one placed in our JS utility.js that monitors for content changes and sets a flag that is exposed globally to parent tabs or form(perhaps a session variable) that can be checked by alert before popping up. Set the context for the changes also.

stu01509 · 2020-04-02T07:42:33Z

Hi @sjpadgett @stephenwaite and @bradymiller

I try to in interface/forms/soap/templates/general_new.html write a javascript addEventListener to record text change counter, But it will happen open SOAP page HTTP 500 error.

Also, I've tried in utility.js writer a counter to record, But It will happen redeclaration of let, Any suggestion or advise?

stephenwaite · 2020-04-03T13:13:28Z

hi @stu01509 , not really sure, would you like to push you're best effort so can take a look at the code?

stu01509 · 2020-04-03T22:39:42Z

Hi @sjpadgett @stephenwaite and @bradymiller

Finally, I used local storage to record isSoapEdit variables when SOAP textarea trigger onkeyup event. Now the modal only shows when the textarea has modified.

Please review it again, Thanks.

stephenwaite · 2020-04-04T23:41:46Z

nice @stu01509 , that fixes when you click to close the tab but not the cancel button, thank you

sjpadgett · 2020-04-04T23:55:23Z

In form cancel will require you to create a doCancel() function to replace the window.location redirect in the cancel button. The check you onchange flag and act accordingly. Just saying:)

stu01509 · 2020-04-05T03:01:01Z

Thanks for @stephenwaite and @sjpadgett review and suggestion,
I will try to improve this solution and push a new commit asap.

bradymiller · 2020-04-05T03:11:45Z

src/Tabs/TabsWrapper.php

+        const panelId = self.parent().attr("href").substring(1);
+        top.restoreSession();
+        twCloseTab(tabsid, panelId);
+        localStorage.removeItem('isSoapEdit');


shouldn't need to use localStorage. would be more straightforward to just store it in top.* . See here for example of things that are stored there:
https://github.com/openemr/openemr/blob/master/interface/main/tabs/main.php#L157-L164

Wow, Thanks for your instruction, It's very useful to me,
I will try it! 😁

stu01509 · 2020-04-05T06:01:02Z

I've tried in general _new.html onkeyup event to trigger console.log(jsGlobals) variables, But it seems can't access this variable.

stu01509 · 2020-04-05T06:52:22Z

Now, I still use localstorage to record the isSoapEdit variable, Because of the general _new.html page can't access jsGlobals variable.

Any suggestions or feedback to improve it?

bradymiller · 2020-04-06T03:23:28Z

hi @stu01509 , To store the variable in top, you would end up doing something like:

top.isSoapEdit = true;

stu01509 · 2020-04-06T12:28:57Z

Oh my! It's working.
Please review it again 😁

stephenwaite · 2020-04-07T01:23:58Z

hi @stu01509 , the alert works great when you try to close the tab but there's still no alert for clicking the Cancel button. Not seeing anything new in the onclick attr of that button. Thank you

bradymiller · 2020-04-07T09:09:41Z

interface/forms/soap/templates/general_new.html

+            if (top.isSoapEdit === true) {
+                dlgopen('', '', 450, 125, '', '<div class="text-danger">Warning</div>', {
+                    type: 'Alert',
+                    html: '<p>Do you want to close the tabs?</p>',


translate (using the above xla construct)

bradymiller · 2020-04-07T09:10:14Z

interface/forms/soap/templates/general_new.html

+                    html: '<p>Do you want to close the tabs?</p>',
+                    buttons: [
+                        {text: 'Cancel', close: true, style: 'default btn-sm'},
+                        {text: 'Close', close: true, style: 'danger btn-sm', click: closeSoap},


translate above 2 labels (using the above xla construct)

bradymiller · 2020-04-07T09:11:43Z

src/Tabs/TabsWrapper.php

+    }
+
+    if (self[0].id === 'SOAP' && top.isSoapEdit === true) {
+        dlgopen('', '', 450, 125, '', '<div class="text-danger">Warning</div>', {


translate (note this is different than in above smarty context):

dlgopen('', '', 450, 125, '', '<div class="text-danger"><?php echo xla("Warning"); ?></div>', {

Hi @bradymiller

Here, The title will disappear, I guessing in here it using <<<EOD and EOD

man, EOD...
your gonna have to look up how to call php functions in EOD (there may be examples of this in OpenEMR codebase, but not sure)

Sounds like a huge project 🤣, I will try to fix it.
Thank you.

bradymiller · 2020-04-07T09:12:08Z

src/Tabs/TabsWrapper.php

+    if (self[0].id === 'SOAP' && top.isSoapEdit === true) {
+        dlgopen('', '', 450, 125, '', '<div class="text-danger">Warning</div>', {
+            type: 'Alert',
+            html: '<p>Do you want to close the tabs?</p>',


translate (using the above xla function)

bradymiller · 2020-04-07T09:12:29Z

src/Tabs/TabsWrapper.php

+            html: '<p>Do you want to close the tabs?</p>',
+            buttons: [
+                {text: 'Cancel', close: true, style: 'default btn-sm'},
+                {text: 'Close', close: true, style: 'danger btn-sm', click: closeSoap},


translate above 2 labels (using the above xla function)

stu01509 · 2020-04-07T09:25:59Z

Hi @bradymiller

I try to change 'general_new.html'
to dlgopen('', '', 450, 125, '', '<div class="text-danger">{xla t="Warning"}</div>', {

But it looks like have some problem, I guessing maybe is {literal} tag effect?

stephenwaite · 2020-04-07T15:49:49Z

think so @stu01509 , try the {literal}, https://www.smarty.net/docs/en/language.function.literal.tpl

stu01509 · 2020-04-08T12:45:21Z

Hi @stephenwaite
Thanks for your information, Let me fix translate in EOD problems :)

stu01509 · 2020-04-08T13:07:46Z

Hi @bradymiller and @stephenwaite

Updated this commit, I used variables to save translate content.
Please review it again, Thanks.

stephenwaite · 2020-04-08T15:18:34Z

hi @stu01509 , nice work! guess this can be extended to other forms which would build on this and be a larger project? would you like to create the parent project and reference this? thank you

bradymiller · 2020-04-08T17:48:12Z

src/Tabs/TabsWrapper.php

@@ -113,9 +113,13 @@ public static function genJavaScript()
        $s = '';
        if (!defined('INCLUDED_TW_ONETIME_JS')) {
            define('INCLUDED_TW_ONETIME_JS', true);
+            $modalTitle = xla("Warning");
+            $modalContent = xla("Do you want to close the tabs");


place the punctuation (?) within the constant

Done! Please review it again :)

bradymiller · 2020-04-08T17:50:32Z

hi @stu01509 , What will happen here if a user opens up two SOAP forms at the same time and works on them?

stu01509 · 2020-04-08T20:53:52Z

Hello @bradymiller

If a user opens up two SOAP forms, and just edit one of form when it closes the anyone form, just show modal dialog.

stu01509 · 2020-04-08T21:13:10Z

Hello @stephenwaite

Sounds good ideas, I would like to build on this project after this PR merged, Can you open a new issue and assign me? Thanks 😁

sjpadgett · 2020-04-08T21:30:42Z

This is why that global js should be array that includes context. This makes life so much easier.
In top:
var FormAction = [{id:'form id'}, {context:'edit'}, {name:'tab name'}, {status:'changed'}];

then universal function to deal.

Just saying one var should handle all edits and just a suggestion from the busy body that is me:)

stu01509 · 2020-04-08T22:34:54Z

Hi @sjpadgett

Thanks for your suggestion 👍, It's a good idea to deal with all the encounter form status, I will plan it in the next feature.

sjpadgett · 2020-04-08T23:34:15Z

It's a good idea to deal with all the encounter form status

Okay but I collect royalties! $9.99 ought to do it:)

stu01509 · 2020-04-09T04:20:56Z

royalties

Okay, Let me use PR to instead of royalties 😁

bradymiller · 2020-04-09T06:56:25Z

hi @stu01509 , Testing well and bringing into codebase. Great job persevering on this one and agree that a generic mechanism that leverages this (ie. any form can plug into it) would be very nice. Thanks for the improvement! -brady

stu01509 · 2020-04-09T09:41:15Z

Hello @bradymiller

Let me planing a pretty plan for implementation about this feature, and that can easily to use in any form and show the dialog 😃

stu01509 added 2 commits April 1, 2020 18:24

feat: add close tabs warning dialog

00b05cc

feat: add close tabs warning dialog

aba274f

bradymiller added Status: Needs Review Status: Needs Work labels Apr 1, 2020

fix: use localstorage to record isSoapEdit

4a133e0

bradymiller removed the Status: Needs Work label Apr 4, 2020

stu01509 changed the title ~~Fix #3279 warning closing form~~ [WIP]Fix #3279 warning closing form Apr 4, 2020

stephenwaite added Status: Needs Work and removed Status: Needs Review labels Apr 4, 2020

bradymiller reviewed Apr 5, 2020

View reviewed changes

fix: warning modal logic bug

32ccabb

stu01509 changed the title ~~[WIP]Fix #3279 warning closing form~~ Fix #3279 warning closing form Apr 5, 2020

bradymiller added the Status: Needs Review label Apr 6, 2020

fix: change localstorage to js top variable

ad1b1d8

stephenwaite removed the Status: Needs Review label Apr 7, 2020

bradymiller reviewed Apr 7, 2020

View reviewed changes

fix: add modal translate

05e7178

stephenwaite added Status: Needs Review Status: Ready for Integration and removed Status: Needs Work Status: Needs Review labels Apr 8, 2020

bradymiller reviewed Apr 8, 2020

View reviewed changes

bradymiller added the Status: Needs Work label Apr 8, 2020

fix: adjust variables

b0859c8

bradymiller merged commit 0544f5c into openemr:master Apr 9, 2020

stu01509 mentioned this pull request Apr 12, 2020

Modifying/customizing existing forms #2921

Closed

stephenwaite mentioned this pull request May 27, 2020

Closing a tab not warning of unsaved changes #290

Open

Fix #3279 warning closing form #3303

Fix #3279 warning closing form #3303

Conversation

stu01509 commented Apr 1, 2020

Short description of what this resolves:

stu01509 commented Apr 1, 2020

sjpadgett commented Apr 1, 2020

stu01509 commented Apr 1, 2020

sjpadgett commented Apr 1, 2020

stu01509 commented Apr 2, 2020 • edited

stephenwaite commented Apr 3, 2020

stu01509 commented Apr 3, 2020 • edited

stephenwaite commented Apr 4, 2020

sjpadgett commented Apr 4, 2020

stu01509 commented Apr 5, 2020

bradymiller Apr 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stu01509 commented Apr 5, 2020

stu01509 commented Apr 5, 2020 • edited

bradymiller commented Apr 6, 2020

stu01509 commented Apr 6, 2020

stephenwaite commented Apr 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stu01509 commented Apr 7, 2020

stephenwaite commented Apr 7, 2020

stu01509 commented Apr 8, 2020

stu01509 commented Apr 8, 2020

stephenwaite commented Apr 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradymiller commented Apr 8, 2020

stu01509 commented Apr 8, 2020 • edited

stu01509 commented Apr 8, 2020

sjpadgett commented Apr 8, 2020

stu01509 commented Apr 8, 2020

sjpadgett commented Apr 8, 2020

stu01509 commented Apr 9, 2020

bradymiller commented Apr 9, 2020

stu01509 commented Apr 9, 2020

stu01509 commented Apr 2, 2020 •

edited

stu01509 commented Apr 3, 2020 •

edited

bradymiller Apr 5, 2020 •

edited

stu01509 commented Apr 5, 2020 •

edited

stu01509 commented Apr 8, 2020 •

edited