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

Fix W3C validator issues #5201

Merged
merged 7 commits into from
Sep 19, 2019
Merged

Fix W3C validator issues #5201

merged 7 commits into from
Sep 19, 2019

Conversation

sergejmueller
Copy link
Contributor

Description

Following issues are reported by the W3C validator and have been fixed.

  1. Remove unused double quotes
  2. Remove duplicate meta description
  3. Remove unnecessary type for JS
  4. Remove useless async attribute from script
  5. Move footer includes into body tag
  6. Add missign alt attributes
Testing

✓ Frontend tests

The type attribute is unnecessary for JavaScript resources.
Element script must not have attribute async unless attribute src is also specified.
An img element must have an alt attribute.
@codecov
Copy link

codecov bot commented Sep 16, 2019

Codecov Report

Merging #5201 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5201   +/-   ##
=======================================
  Coverage   30.81%   30.81%           
=======================================
  Files         221      221           
  Lines       17804    17804           
  Branches     2457     2457           
=======================================
  Hits         5487     5487           
  Misses      12091    12091           
  Partials      226      226

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8956dd...2d8dd75. Read the comment docs.

Copy link
Contributor

@danlipert danlipert left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for these fixes! Just a few questions I had but looks good to me!

app/retail/templates/shared/analytics.html Show resolved Hide resolved
app/retail/templates/shared/cards.html Show resolved Hide resolved
{% include 'shared/analytics.html' %}
{% include 'shared/footer_scripts_lite.html' with slim=1 %}
{% include 'shared/footer.html' %}
{% include 'shared/messages.html' %}
Copy link
Contributor

@octavioamu octavioamu Sep 18, 2019

Choose a reason for hiding this comment

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

good maybe a good idea to move the other scripts behind this comment also inside of body

Copy link
Contributor Author

@sergejmueller sergejmueller Sep 18, 2019

Choose a reason for hiding this comment

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

Actually yes, but I have based myself on existing webpages:

{% include 'shared/bottom_notification.html' %}
{% include 'shared/analytics.html' %}
{% include 'shared/footer_scripts.html' %}
{% include 'shared/footer.html' %}
{% include 'shared/messages.html' %}
</body>
<script>
document.is_funder_github_user_same = {{ is_funder }};
document.FEE_PERCENTAGE = {{ FEE_PERCENTAGE }};
</script>
<script src="{% static "v2/js/pages/wallet_estimate.js" %}"></script>
<script src="{% static "v2/js/lib/ipfs-api.js" %}"></script>
<script src="{% static "v2/js/ipfs.js" %}"></script>
<script src="{% static "v2/js/lib/secrets.min.js" %}"></script>
<script src="{% static "v2/js/ethereumjs-accounts.js" %}"></script>
<script src="{% static "v2/js/lib/ipfs-api.js" %}"></script>
<script src="{% static "v2/js/ipfs.js" %}"></script>
<script src="{% static "v2/js/amounts.js" %}"></script>
<script src="{% static "v2/js/abi.js" %}"></script>
<script src="/dynamic/js/tokens_dynamic.js"></script>
<script src="{% static "v2/js/tokens.js" %}"></script>
<script src="{% static "v2/js/pages/shared_bounty_mutation_estimate_gas.js" %}"></script>
<script src="{% static "v2/js/pages/increase_bounty.js" %}"></script>
<script src="{% static "onepager/js/send.js" %}"></script>
</html>

{% include 'shared/bottom_notification.html' %}
{% include 'shared/analytics.html' %}
{% include 'shared/footer_scripts.html' %}
{% include 'shared/footer.html' %}
{% include 'grants/shared/shared_scripts.html' %}
</body>
<script src="{% static "v2/js/pages/wallet_estimate.js" %}"></script>
<script src="{% static "v2/js/grants/compiledSplitter.js" %}"></script>
<script src="{% static "v2/js/grants/compiledSubscriptionContract0.js" %}"></script>
<script src="{% static "v2/js/grants/compiledSubscriptionContract1.js" %}"></script>
<script src="{% static "v2/js/grants/compiledTokenContract.js" %}"></script>
<script src="{% static "v2/js/abi.js" %}"></script>
<script src="{% static "v2/js/pages/shared_bounty_mutation_estimate_gas.js" %}"></script>
<script src="{% static "v2/js/lib/ipfs-api.js" %}"></script>
<script src="{% static "v2/js/ipfs.js" %}"></script>
<script src="{% static "v2/js/grants/shared.js" %}"></script>
<script src="{% static "v2/js/waiting_room_entertainment.js" %}"></script>
<script src="{% static "v2/js/grants/cancel_subscription.js" %}"></script>
</html>

{% include 'shared/bottom_notification.html' %}
{% include 'shared/analytics.html' %}
{% include 'shared/footer_scripts.html' %}
{% include 'shared/footer.html' %}
{% include 'grants/shared/shared_scripts.html' %}
{% if show_tweet_modal %}
<script>
$(document).ready(function(){
$('#tweetModal').css('display', 'block');
});
</script>
{% endif %}
</body>
<script src="{% static "v2/js/pages/wallet_estimate.js" %}"></script>
<script src="{% static "v2/js/grants/compiledSplitter.js" %}"></script>
<script src="{% static "v2/js/grants/compiledSubscriptionContract0.js" %}"></script>
<script src="{% static "v2/js/grants/compiledSubscriptionContract1.js" %}"></script>
<script src="{% static "v2/js/grants/compiledTokenContract.js" %}"></script>
<script src="/dynamic/js/tokens_dynamic.js"></script>
<script src="{% static "v2/js/tokens.js" %}"></script>
<script src="{% static "v2/js/abi.js" %}"></script>
<script src="{% static "v2/js/pages/shared_bounty_mutation_estimate_gas.js" %}"></script>
<script src="{% static "v2/js/grants/shared.js" %}"></script>
<script src="{% static "v2/js/grants/fund.js" %}"></script>
<script src="{% static "v2/js/lib/ipfs-api.js" %}"></script>
<script src="{% static "v2/js/ipfs.js" %}"></script>
<script src="{% static "v2/js/waiting_room_entertainment.js" %}"></script>
</html>

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, yes good point we should fix that in some point (not for this PR)

@thelostone-mc thelostone-mc merged commit 9e3940a into gitcoinco:master Sep 19, 2019
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 this pull request may close these issues.

5 participants