-
Notifications
You must be signed in to change notification settings - Fork 9
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
38334 Only load required scripts and CSS #610
Conversation
Pull Request Test Coverage Report for Build 6482798248
💛 - Coveralls |
@yeneastgate or @dai-eastgate could you merge master into this bracne. I think it will show some merge conflicts. |
I will merge master into this branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will start testing.
Steps to install the approved version:
|
While testing I get a TypeError: $ is not a function. I don't know why this happens on my W3PM test enviorement. Because Jquery works well on another onOffice-Demo enviorement. Error on Estatelist with multi select fields in search: No Error on Estatelist with multi select fields in search: I double check right pluginversion -> both Version 4.14-18-gc61278ec) ✔ Maybe you habe an idea? @dai-eastgate @yeneastgate ? |
@andernath I will check it. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeat testing.
Steps to install the approved version:
|
@dai-eastgate |
I will check it. Thanks! |
@dai-eastgate will you implement leaflet loading or is it part of another issue? |
@andernath I'm implementing "leaflet loading". I will respond to you by the end of today. Because this feature works a bit differently, I had to check it more carefully |
Good Job! Thank you :) |
@andernath @fredericalpers I have found a solution that loads the "leaflet" library only when needed and puts it in branch "38334-only-load-required-scripts-and-css-v2". leaflet.mp4However, this solution will affect backward compatibility. In my personal opinion, I suggest keeping the "leaflet" library loaded in the tag and loading it every time like the current master branch. |
@dai-eastgate I see your struggle :/ Can I continue testing here in #610 or did I miss any changes on "38334-only-load-required-scripts-and-css-v2" wich needed to be merged in here? |
@andernath OK, thanks. I will keep loading leaflet always |
@andernath I updated the code in #610. Please review it again. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Start Testing
Steps to install the approved version:
|
related to #554
changed log :
Update load the js file in the right place