get dialog to close on Save Changes for groups #6792

[pdurbin]

What this PR does / why we need it:

When you click "Save Changes" on the groups page, the dialog doesn't close.

Which issue(s) this PR closes:

Closes #6792

Special notes for your reviewer:

This reverts pull request #6023 (f40d653 specifically) so I'm concerned #5967 will resurface. Mostly I'm just putting this pull request out there for discussion, so see if others have ideas about how to fix this.

Suggestions on how to test this:

Edit a group and click Save Changes. The dialog should close.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No, apart from a dialog closing properly. It looks like this:

Is there a release notes update needed for this change?:

Yes but I haven't added one yet because I'm not sure this is the fix we want.

Additional documentation:

None.

[coveralls]

Coverage decreased (-0.005%) to 19.481% when pulling 79301e6 on 6792-manage-groups into 922b0cd on develop.

[pdurbin]

As an alternative fix, I tried moving the dialog down (diff below) but it didn't help. The only way I've gotten "Save Changes" to work properly is with the (admittedly controversial!) CDATA removal in e0fc1f8 (this pull request).

beamish:dataverse pdurbin$ git diff -U5
diff --git a/src/main/webapp/manage-groups.xhtml b/src/main/webapp/manage-groups.xhtml
index 706c8f7ea..91c94d807 100644
--- a/src/main/webapp/manage-groups.xhtml
+++ b/src/main/webapp/manage-groups.xhtml
@@ -94,10 +94,12 @@
                         <div class="button-block">
                             <p:commandButton styleClass="btn btn-default" value="#{bundle.continue}" onclick="PF('deleteConfirmation').hide()" action="#{manageGroupsPage.deleteGroup()}" update="@all"/>
                             <p:commandButton styleClass="btn btn-link" value="#{bundle.cancel}" onclick="PF('deleteConfirmation').hide()" action="#{manageGroupsPage.unrenderDeletePopup()}" update="@all"/>
                         </div>
                     </p:dialog>
+                    <!-- Explicit Group Edit Popup -->
+                    <ui:include src="explicitGroup-new-dialog.xhtml"/>
                     <p:dialog id="viewGroup" styleClass="largePopUp" header="#{bundle['dataverse.manageGroups.tab.action.btn.view.dialog.header']}" widgetVar="viewGroup" modal="true">
                         <p:focus context="viewGroup"/>
                         <div class="form-horizontal">
                             <div class="form-group">
                                 <label for="select_GroupName" class="col-sm-3 control-label">
@@ -176,12 +178,10 @@
                             <button class="btn btn-link" onclick="PF('viewGroup').hide()" type="button">
                                 #{bundle.cancel}
                             </button>
                         </div>
                     </p:dialog>
-                    <!-- Explicit Group Edit Popup -->
-                    <ui:include src="explicitGroup-new-dialog.xhtml"/>
                 </h:form>
             </ui:define>
         </ui:composition>
     </h:body>
 </html>

[qqmyers]

Given that the problem appears to be linked to having the script in the .html, which is included in the .xhtml and causing issues due to the > and & chars in the scripts (as far as we know), maybe pulling it out to a separate .js file and linking that would resolve this and avoid the earlier issue?

[scolapasta]

Looks good, though I think incomplete. (I also added a cleanup request that I think would make sense to consider for this PR. Lastly, do you want to add a release note? I don't it needs one as it's a simple bug fix, but that's up to you.

[pdurbin]

Ok, after addressing some concerns in 79301e6, I'm ready for more code review.

[scolapasta]

Looks good!