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.

Sign up for GitHub

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

replace leaflet files from assets.rb in questionRich.html and _mapDependencies.html #7945

Merged
merged 3 commits into from
Aug 18, 2020
Merged

replace leaflet files from assets.rb in questionRich.html and _mapDependencies.html #7945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad8881f
replace leaflet files from assets.rb in questionRich.html and _mapDep…
Tlazypanda May 22, 2020
756d20b
Merge branch 'master' into leaflet_questionRich_script
Tlazypanda Jun 29, 2020
d8f7231
Merge branch 'main' into leaflet_questionRich_script
Tlazypanda Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/assets/stylesheets/application.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@
*= require location
*= require notes
*= require spam2
*/
*/
10 changes: 2 additions & 8 deletions app/views/editor/questionRich.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,15 @@
<link href="/lib/publiclab-editor/dist/PublicLab.Editor.css" rel="stylesheet">

<!-- required for MapModule -->
<link href="/lib/leaflet/dist/leaflet.css" rel="stylesheet">
<script src="/lib/leaflet/dist/leaflet.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep this and remove from application level, because as you are also working on performance improvement, having a light entry page should take less time, right?
When these libraries are required they should be loaded only then (lazy loading), what do you think?
😄 🎉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Tlazypanda any comment on these last questions from @sagarpreet-chadha ? My apologies for replying late to these. Thank you!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jywarren @sagarpreet-chadha I totally missed this 😅 my apologies ...ummm can we do this in a follow up pr because I think the leaflet.js and css files are actually required on many routes since the map is loaded on dashboard/user profile/all editor pages/map routes etc so might have to investigate it more to make sure that removing it won't break anything. What do you think? Thanks ✌️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's totally fine. Always game to split up PRs! Does that make this PR otherwise good to merge? Thank you!

<link href="/lib/leaflet-blurred-location/dist/Leaflet.BlurredLocation.css" rel="stylesheet">
<script src="/lib/leaflet-blurred-location/dist/Leaflet.BlurredLocation.js"></script>
<%= stylesheet_link_tag "leaflet-blurred-location/dist/Leaflet.BlurredLocation.css" %>
<%= javascript_include_tag('leaflet-blurred-location/dist/Leaflet.BlurredLocation.js') %>
<div id="map" class="leaflet-map" style="width: 100% ; height: 300px; margin-bottom:8px;display:none;"></div>

<!-- required for TagsModule -->
<script src="/lib/typeahead.js/dist/bloodhound.js"></script>
<script src="/lib/bootstrap-tokenfield/dist/bootstrap-tokenfield.js"></script>

<!-- required for MainImageModule -->
<script src="/lib/jquery-file-upload/js/vendor/jquery.ui.widget.js"></script>
<script src="/lib/jquery-file-upload/js/jquery.iframe-transport.js"></script>
<script src="/lib/jquery-file-upload/js/jquery.fileupload.js"></script>

<script src="/lib/publiclab-editor/dist/PublicLab.Editor.js"></script>

<div class="alert alert-success" style="width: 100%;" >This is the new rich Questions form. <a href="/questions/new?legacy=true">Click here for the legacy form</a>.</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/map/_mapDependencies.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

<%= stylesheet_link_tag "/lib/leaflet-environmental-layers/dist/LeafletEnvironmentalLayers.css" %>

<%= javascript_include_tag('/lib/leaflet-blurred-location/dist/Leaflet.BlurredLocation.js') %>
<%= javascript_include_tag('leaflet-blurred-location/dist/Leaflet.BlurredLocation.js') %>
<%= javascript_include_tag('/lib/leaflet.blurred-location-display/dist/Leaflet.BlurredLocationDisplay.js') %>

<%= javascript_include_tag('/lib/leaflet-environmental-layers/dist/LeafletEnvironmentalLayers.js') %>
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/assets.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Rails.application.config.assets.precompile += [
'leaflet-blurred-location/dist/Leaflet.BlurredLocation.js',
'leaflet-blurred-location/dist/Leaflet.BlurredLocation.css',

'advanced_search.js',
'submit_form_ajax.js',
'application.js',
Expand Down