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

b:dataTable causes JavaScript errors in Liferay 7.0 due to CDN resource #853

Closed
stiemannkj1 opened this issue Sep 1, 2017 · 16 comments
Closed
Assignees
Milestone

Comments

@stiemannkj1
Copy link
Contributor

Liferay 7.0 exposes an AMD loader and certain JS resources do no behave well when an AMD loader is present due to anonymous modules. In order to work around these errors, the Liferay Faces Bridge filters the content of these resources slightly to disable their AMD loader features. datatables.min.js is one of the resources that we must filter. With BootsFaces v1.1.2 things worked fine, but with v1.1.3 datatables.min.js was moved to using a CDN. Since datatables.min.js is no longer present on the server, the Liferay Faces custom ResourceHandler can no longer filter the datatables.min.js resource to make it compatible with Liferay 7.0's frontend JS.

In order to fix this, datatables.min.js should be included in the BootsFaces jar (or a ResLib jar) as it was in v1.1.2 and an option should probably be included to enable loading the resource from a CDN.

@chsreedhar
Copy link

@stephanrauh, include datatables.min.js and datatables.min.css in to jar, otherwise <b:dataTable /> not going to work without internet connection.

@stephanrauh
Copy link
Collaborator

@chsreedhar We've extracted the file because it adds a huge amount of memory to the core library. However, we can split the BootsFaces.jar into two jars: one containing the core library, and an optional jar containing the datatable files. If the optional library is not there, we use the CDN approach. How do you like this idea?

@chsreedhar
Copy link

@stephanrauh nice idea.

@paulohva
Copy link

@stephanrauh Is this enhancement planned to be in upcoming releases? I can't update my version of Bootsfaces without a solution for this.

@stephanrauh
Copy link
Collaborator

@TheCoder4eu What's your opinion?

@NMagarreiro
Copy link

@stephanrauh and @TheCoder4eu, there are any news about this issue? Our client machines does not have internet connection, so we need this issue resolved in order to upgrade BootsFaces version.

@NMagarreiro
Copy link

Hi @stephanrauh and @TheCoder4eu,
I checked that this problem still happens on BootsFaces 1.3.0.
Is this correction planned to a future release?

Thanks.

@stiemannkj1
Copy link
Contributor Author

stiemannkj1 commented Oct 4, 2018

@stephanrauh, @TheCoder4eu, could you make the URL for the resource configurable via init-param so that people could provide their own local version if they want?

@stephanrauh
Copy link
Collaborator

stephanrauh commented Oct 4, 2018

@stiemannkj1 @chsreedhar @NMagarreiro @paulohva First of all, sorry for forgetting this ticket. Somehow, it got buried under a pile of new tickets.

But maybe it's good we didn't implement the solution in a hurry. Kyle, I like your idea better than my original idea. Maybe you're familiar with our approach to using jQuery? I'd like to implement the same idea for the datatable:

  • BootsFaces checks the header and the body for JS and CSS resources. If it finds something containing the keyword "datatables" and ending with either ".js" or ".css", we assume the user brings their own copy of datatables.min.{css|js}.
  • We also add a web.xml parameter stopping BootsFaces from loading the datatable resources. I suppose nobody needs such a feature, but you never know.

How do you like the idea?

@stephanrauh stephanrauh added this to the v1.5.0 milestone Oct 4, 2018
@stephanrauh stephanrauh self-assigned this Oct 4, 2018
@stiemannkj1
Copy link
Contributor Author

@stephanrauh, sounds good to me. Perhaps you could include an example or documentation showing how to use this feature as part of the ticket?

@stephanrauh
Copy link
Collaborator

@stiemannkj1 Yes, sure - but give me some time :). I've pushed a tentative implementation to GitHub. It passed my first few tests, but even so, take it with a grain of salt. If you're ready to dive into the sourcecode (it's a fairly small change), follow the link above.

@stiemannkj1
Copy link
Contributor Author

@stephanrauh, looks good :) Minor tweaks:

You may be able to skip the looping check if get_datatable_from_cdn == false.
You may be able to abort the loop (break) when loadDatatables is set to false.

@stephanrauh
Copy link
Collaborator

@stiemannkj1 @chsreedhar @paulohva @NMagarreiro I can't decided how to name the context parameter. Is it more intuitive to call it net.bootsfaces.get_datatable_from_cdn? Or should we use the plural form, because the JS widget is called DataTables.net?

IMHO we should prefer the singular form because that's the name BootsFaces uses (<b:dataTable>). But it feels a bit odd to ignore the naming pattern of the underlying JS widget, especially taking into account how much work and passion they've invested in their pet framework. What's your opinion?

stephanrauh added a commit to TheCoder4eu/BootsFacesWeb that referenced this issue Oct 5, 2018
@stephanrauh
Copy link
Collaborator

Current state of the art: there's some documentation, but the live demo is still being developed. What's already there should appear soon at https://showcase.bootsfaces.net/layout/resourcemanagement.jsf.

@chsreedhar
Copy link

@stephanrauh use context parameter net.bootsfaces.get_datatable_from_cdn and go ahead with your proposal above.

@stephanrauh
Copy link
Collaborator

I've finished the first demo at https://staging.bootsfaces.net/Showcase/layout/resourcemanagement.jsf.

Currently, the demo only demonstrates how to use the web.xml parameter. The other detection mechanisms still need a demo in the showcase.

stephanrauh added a commit that referenced this issue Oct 8, 2018
stephanrauh added a commit that referenced this issue Oct 8, 2018
stephanrauh added a commit that referenced this issue Oct 8, 2018
stephanrauh added a commit to TheCoder4eu/BootsFacesWeb that referenced this issue Oct 8, 2018
stephanrauh added a commit that referenced this issue Oct 8, 2018
stephanrauh added a commit that referenced this issue Oct 10, 2018
stephanrauh added a commit to TheCoder4eu/BootsFacesWeb that referenced this issue Oct 11, 2018
stephanrauh added a commit to TheCoder4eu/BootsFacesWeb that referenced this issue Oct 11, 2018
stephanrauh added a commit to TheCoder4eu/BootsFacesWeb that referenced this issue Oct 11, 2018
stephanrauh added a commit that referenced this issue Oct 13, 2018
stephanrauh added a commit that referenced this issue Oct 13, 2018
stephanrauh added a commit that referenced this issue Oct 13, 2018
stephanrauh added a commit that referenced this issue Oct 13, 2018
stephanrauh added a commit that referenced this issue Oct 13, 2018
@stephanrauh stephanrauh modified the milestones: v1.5.0, v1.4.0 Oct 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants