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

Improve (simplify) Primefaces (Ajax) / Bootstrap interaction #7565

Closed
scolapasta opened this issue Jan 29, 2021 · 10 comments · Fixed by #8586 or #8627
Closed

Improve (simplify) Primefaces (Ajax) / Bootstrap interaction #7565

scolapasta opened this issue Jan 29, 2021 · 10 comments · Fixed by #8586 or #8627
Assignees
Milestone

Comments

@scolapasta
Copy link
Contributor

With Bootstrap, we do a lot of "binding" of html elements, with javascript, e.g. tooltips or popup previews for image files. This is done by calling a JS function bind_bsui_components when we ready the document.

With Primefaces, we do a lot of ajax calls that update parts of pages - when that happens the bootstrap binding is lost. This is because the update replaces the html, but doesn't know to call the rebinding.

We currently solve this by calling the javascript for each update with oncomplete="bind_bsui_components"

This is prone to error, because we have to remember to put it everywhere.

@scolapasta
Copy link
Contributor Author

So, I opened this issue because I tried an experiment that worked! I think it needs some more testing / refinement, but something of it's nature should eliminate the need to include oncomplete="bind_bsui_components" for all updates.

In dataverse_template.xhtml:
add:

      <p:fragment>
        <p:autoUpdate/>
         ...
     </p:fragment>

around the <script> at the end of the page.

That said, @mheppler are a little unsure if why it works, as we weren't sure if the document.ready would or should get called when that part of the page us updated. So it would be good to understand better why this is working and make sure there's no harm in calling other aspects.

(in doing some other research, there's another thing i want to try and will report back on shortly.

@scolapasta
Copy link
Contributor Author

OK, tried the other thing I had found:
https://api.jquery.com/ajaxComplete/

But it did not seem to work as I expected. More experimentation to be done, especially with the first solution, which seems to work.

@scolapasta
Copy link
Contributor Author

scolapasta commented Jan 29, 2021

I keep seeing things like:
"The $(document).ready() event fires once, when the DOM has fully loaded from the initial load. An ajax event may change the DOM, but it doesn't reload the page, so the $(document).ready() won't fire again."

In the experiment above, though, we are effectively redefining the document.ready by updating the part of the page that contains it, and that does trigger it to fire. But I have yet to find any agreement (or disagreement) that this is an ok approach.

That said, I may have found a more "accepted" way. Omnifaces has an o:onloadScript component that will get run after ajax calls:
https://stackoverflow.com/questions/14400624/jsf-primefaces-ajax-updates-breaks-jquery-event-listener-function-bindings
https://showcase.omnifaces.org/components/onloadScript

I tried it, and it seems to work as well; just added:

        <o:onloadScript>bind_bsui_components();</o:onloadScript>       

to the dataset_template.xhtml (and removed the bind call from the documnet.ready).

@scolapasta
Copy link
Contributor Author

So to be clear, I think this issue is ready to be worked on:
• add the onLoadScript
• clean up the oncompletes

@scolapasta scolapasta changed the title Improve (simplify) Primefaces / Bootstrap interaction Improve (simplify) Primefaces (Ajax) / Bootstrap interaction Jan 29, 2021
@mheppler
Copy link
Contributor

mheppler commented Feb 11, 2021

Lots to clean up...

Screen Shot 2021-02-11 at 9 50 42 AM

  • dataset-license-terms.xhtml
  • dataset-widgets.xhtml
  • dataset.xhtml
  • dataverse.xhtml
  • dataverse_template.xhtml
  • dataverseuser.xhtml
  • editFilesFragment.xhtml
  • editdatafiles.xhtml
  • file-edit-button-fragment.xhtml
  • file.xhtml
  • filesFragment.xhtml
  • guestbook.xhtml
  • harvestclients.xhtml
  • harvestsets.xhtml
  • manage-groups.xhtml
  • manage-guestbooks.xhtml
  • manage-templates.xhtml
  • metadataFragment.xhtml
  • permissions-manage-files.xhtml
  • permissions-manage.xhtml
  • provenance-popups-fragment.xhtml
  • themeAndWidgetsFragment.xhtml

Found some inventory documents (doc, spreadsheet) that might be able to help track the various features, across all the pages.

@mheppler
Copy link
Contributor

Related code clean up Infrastructure Spring Cleaning - Remove misc files #5348.

@mheppler
Copy link
Contributor

mheppler commented Mar 5, 2021

Added first official use of onloadScript to the dataset pg as part of Truncate methods: checksums, long summary metadata fields #6685.

<o:onloadScript>summaryDescTruncation();</o:onloadScript>

@djbrooke
Copy link
Contributor

djbrooke commented Mar 10, 2021

  • The failure case is likely not breaking, but potentially ~millisecond slowdown
  • Testing shouldn't involve all the files above - will include some guidelines as part of the PR
  • We should make sure the things in this rebind are there intentionally - suspicion is that there will not be issues
  • Verify that widget views still work (if there are things that do not use the template)

@sekmiller sekmiller self-assigned this Apr 4, 2022
@sekmiller sekmiller moved this from Up Next 🛎 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 4, 2022
sekmiller added a commit that referenced this issue Apr 5, 2022
sekmiller added a commit that referenced this issue Apr 5, 2022
sekmiller added a commit that referenced this issue Apr 6, 2022
@sekmiller
Copy link
Contributor

Keeping this assigned to me and in the project because the linked PR does not update all of the pages in the app.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from IQSS Team - In Progress 💻 to Done 🚀 Apr 14, 2022
@pdurbin pdurbin reopened this Apr 14, 2022
@pdurbin pdurbin moved this from Done 🚀 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 14, 2022
@pdurbin
Copy link
Member

pdurbin commented Apr 14, 2022

Talked to @sekmiller. The pilot is done and merged and he plans to work on the rest so we reopened this issue.

sekmiller added a commit that referenced this issue Apr 15, 2022
sekmiller added a commit that referenced this issue Apr 15, 2022
sekmiller added a commit that referenced this issue Apr 20, 2022
sekmiller added a commit that referenced this issue Apr 20, 2022
sekmiller added a commit that referenced this issue Apr 20, 2022
@sekmiller sekmiller moved this from IQSS Team - In Progress 💻 to Review 🦁 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 20, 2022
sekmiller added a commit that referenced this issue Apr 21, 2022
sekmiller added a commit that referenced this issue Apr 21, 2022
@pdurbin pdurbin added this to the 5.11 milestone May 4, 2022
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

Successfully merging a pull request may close this issue.

5 participants