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

[STALLED] Feature/4172 eliminate render blocking resources #4604

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

[STALLED] Feature/4172 eliminate render blocking resources #4604

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3d275b1
Preload the homepage hero image
rfultz May 6, 2021
a8475f5
Defer JavaScript files
rfultz May 6, 2021
d7bfbb0
Defer other homepage/global scripts
rfultz May 6, 2021
8fb6ebc
Remove commented script tag
rfultz May 18, 2021
5c9c828
cleanURI from homepage template jQuery to ES6 home.js
rfultz May 18, 2021
0e18cd5
Add timer to remove query params for homepage
rfultz May 18, 2021
6fcdf9f
Remove timer, remove params after everything is loaded
rfultz May 20, 2021
46baead
Move the listeners into home.js
rfultz May 20, 2021
d992b00
Move scrubURI from home.js to home_page.html
rfultz May 24, 2021
1eefc42
add win.onload to make sure jquery cleanURI script works with defer o…
May 28, 2021
9ef5020
Fix errors when election-map is on homepage
rfultz Jun 1, 2021
594c179
Another attempt at a native solution
rfultz Jun 1, 2021
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
4 changes: 2 additions & 2 deletions fec/data/templates/election-lookup.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,6 @@ var context = {
}
};
</script>
<script src="{{ asset_for_js('dataviz-common.js') }}"></script>
<script src="{{ asset_for_js('election-lookup.js') }}"></script>
<script defer src="{{ asset_for_js('dataviz-common.js') }}"></script>
<script defer src="{{ asset_for_js('election-lookup.js') }}"></script>
{% endblock %}
4 changes: 2 additions & 2 deletions fec/data/templates/elections.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ var context = {
};

</script>
<script src="{{ asset_for_js('dataviz-common.js') }}"></script>
<script src="{{ asset_for_js('elections.js') }}"></script>
<script defer src="{{ asset_for_js('dataviz-common.js') }}"></script>
<script defer src="{{ asset_for_js('elections.js') }}"></script>
{% endblock %}

4 changes: 2 additions & 2 deletions fec/data/templates/landing.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,6 @@
{% endblock %}

{% block scripts %}
<script src="{{ asset_for_js('dataviz-common.js') }}"></script>
<script src="{{ asset_for_js('data-landing.js') }}"></script>
<script defer src="{{ asset_for_js('dataviz-common.js') }}"></script>
<script defer src="{{ asset_for_js('data-landing.js') }}"></script>
{% endblock %}
4 changes: 2 additions & 2 deletions fec/fec/templates/base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ <h2 style="color: #ffffff">Your browser is outdated</h2>
</script>

{# Global javascript #}
<script type="text/javascript" src="{% asset_for_js 'vendor.js' %}"></script>
<script type="text/javascript" src="{% asset_for_js 'init.js' %}"></script>
<script defer type="text/javascript" src="{% asset_for_js 'vendor.js' %}"></script>
<script defer type="text/javascript" src="{% asset_for_js 'init.js' %}"></script>
<script>
var canSkipPolyfills = ('fetch' in window && 'assign' in Object);
if (!canSkipPolyfills) {
Expand Down
5 changes: 3 additions & 2 deletions fec/fec/templates/home_base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<html lang="en">
<head>
{% include 'partials/meta-tags.html' %}
<link rel="preload" as="image" href="/static/img/Homepage-hero-image-flag.jpg">
{% include 'partials/meta-tags-preloads.html' %}
{% if settings.FEC_CMS_ENVIRONMENT == 'PRODUCTION' %}
{% include 'partials/meta-tags-preconnects.html' %}
Expand Down Expand Up @@ -158,8 +159,8 @@ <h2 style="color: #ffffff">Your browser is outdated</h2>


{# Global javascript #}
<script type="text/javascript" src="{% asset_for_js 'vendor.js' %}"></script>
<script type="text/javascript" src="{% asset_for_js 'init.js' %}"></script>
<script defer type="text/javascript" src="{% asset_for_js 'vendor.js' %}"></script>
<script defer type="text/javascript" src="{% asset_for_js 'init.js' %}"></script>


{% block extra_js %}
Expand Down
41 changes: 21 additions & 20 deletions fec/home/templates/home/home_page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ <h2 id="hero-heading">Protecting the integrity of the campaign finance process</
{% home_page_banner_announcement %}

<section class="slab">
{% comment %}
{% comment %}
Below is for featured icon+links in Wagtail html block only...for now.
{% endcomment %}
{% for block in self.body %}
Expand Down Expand Up @@ -105,31 +105,32 @@ <h1>Commissioners</h1>
if (!canSkipPolyfills) {
var pfScriptElem = document.createElement('script');
pfScriptElem.async = false;
pfScriptElem.defer = false;
pfScriptElem.src = "{% asset_for_js 'polyfills.js' %}";
document.head.appendChild(pfScriptElem);
}
</script>
<script src="{% asset_for_js 'home.js' %}"></script>
<script src="{% asset_for_js 'bythenumbers.js' %}"></script>
<script src="{% asset_for_js 'dataviz-common.js' %}"></script>
<script src="{% asset_for_js 'election-lookup.js' %}"></script>
<script defer src="{% asset_for_js 'home.js' %}"></script>
<script defer src="{% asset_for_js 'bythenumbers.js' %}"></script>
<script defer src="{% asset_for_js 'election-lookup.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.

The cleanURI() function that we use on the homepage to keep the querystring out of the URL does not work if we defer election-lookup.js

It needs jQuery. Specifically, the .on method. I tried to start writing a vanilla JS version, but it gets tricky trying to emulate $.on. Should we replace it with vanilla JS or is there a better way around this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we change election-lookup to async?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to native on another ticket. Should I merge that one into this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I did try async and got same problem

Copy link
Contributor

@johnnyporkchops johnnyporkchops May 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I changed this to native on another ticket. Should I merge that one into this one?

Whatever fixes it is fine with me

<script>
//remove competing/confusing querystrings on homepage
$(function(){
window.onload = function () {
$(function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we'd be using the the native onload event to trigger a jQuery function ($(function)) that creates another function (cleanURI), calls it, and calls four other jQuery commands (on, val, val, and filter.prop)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Not ideal but since it was the only working solution yet, it's worth logging it to the conversation should we need to fall back on it. And only requires adding two lines of code to the existing. Do the latest changes committed since work? Were you able to confirm that before pushing.?

cleanURI()
function cleanURI(){
const uri = window.location.toString();
if (uri.indexOf('?') > 0) {
window.history.replaceState({}, document.title, location.href.split('?')[0]);
function cleanURI(){
const uri = window.location.toString();
if (uri.indexOf('?') > 0) {
window.history.replaceState({}, document.title, location.href.split('?')[0]);
}
}
}
$('#main').on('change ', 'input, select', function(){
cleanURI();
})
//handle Chrome insconsistency with History API
$('.js-office').val('S');
$('.js-election-year').val('2022')
$('.js-chart-toggle').filter('[value=receipts]').prop('checked', true)
});
$('#main').on('change ', 'input, select', function(){
cleanURI();
})
//handle Chrome insconsistency with History API
$('.js-office').val('S');
$('.js-election-year').val('2022')
$('.js-chart-toggle').filter('[value=receipts]').prop('checked', true)
})
}
</script>
{% endblock %}