Separate js from html #785 #1538

curibe · 2021-07-22T17:51:05Z

fixed #785

Separate JS codes from HTML

This PR basically separate a part of JS codes inside two files: base.html and 404_doi.html file to get a more readable HTML file. The JS code extracted was written inside the file scholia.js

Notes
In base.html

Not all JS code was extracted because it requires some jinja statements and variables
The jQuery statement $(document).ready is left inside HTML because It is required by the jinja's macros used to show the result of sparql queries
The others JS code are related with the externalization of SPARQL queries, and it has been resolved in PR 1487 and PR 1517

In 404_doi.html

carlinmack · 2021-07-23T13:13:54Z

Hi, I'm very in favour of this PR :) I would suggest however making workWithQ() into it's own file, and have:

{% if q %}
<script src="{{ url_for('static', filename='workWithQ.js') }}"></script>
{% endif %}

This would help reduce page size (#1502) as not all pages need the full scholia.js. Thoughts?

curibe · 2021-07-26T17:13:35Z

@carlinmack something like this? :

{% if q %}
        
       <script src="{{ url_for('static', filename='workWithQ.js') }}"></script>
	workWithQ("{{ q }}")

	{% if q2 %}
         // The same here
	  workWithQ2("{{ q2 }}")
	{% endif %}
	
	.
        .
        .
{% endif %}

carlinmack · 2021-07-26T17:30:15Z

Hmm sorry just looking at scholia.js now, it seems that most of it is useful for working with Q-number pages? I thought it was more general. In that case it's probably better to go with this PR and then later on see if scholia.js can be dropped from the non-Q-number pages (like the home page).

carlinmack

Will review the other changes once scholia.js is fixed :)

scholia/app/static/scholia.js

carlinmack · 2021-08-07T11:32:55Z

scholia/app/static/scholia.js

+	 try {
+	     var orcid = item.claims.P496[0].mainsnak.datavalue.value;
+	     detailsList.push( '<a href="https://orcid.org/"><img alt="' +
+			       'ORCID logo" src="{{ url_for(\'static\', filename=\'images/orcid_16x16.gif\') }}"' +


This might set this PR back awhile, but you can't use any {{ }} code in the extracted JS. This is because only the HTML is rendered as a Jinja template, replacing these values before being executed in the browser.

I would suggest either dropping the template (as you can do here - replace it with "/static/images/orcid_16x16.gif") or in the case of dynamic stuff like {{ q }}, make it a parameter to the function.

PS, the above results in a GET http://127.0.0.1:8100/author/%7B%7B%20url_for('static',%20filename='images/orcid_16x16.gif')%20%7D%7D 404 (NOT FOUND) error in the console

This might set this PR back awhile, but you can't use any {{ }} code in the extracted JS. This is because only the HTML is rendered as a Jinja template, replacing these values before being executed in the browser.

I would suggest either dropping the template (as you can do here - replace it with "/static/images/orcid_16x16.gif") or in the case of dynamic stuff like {{ q }}, make it a parameter to the function.

Yeah, you right. I think you first recommendation is better because the GIF file is hard-coded

@carlinmack I used a hybrid between the two solutions in case you want to change the static path and also in the case you want to add more files from the static path

Please, let me know what do you think about that.

scholia/app/static/scholia.js

curibe · 2021-08-09T20:59:15Z

scholia/app/templates/base.html

+	// Add anchor links to all headings
+	var headers = document.querySelectorAll('h2[id], h3[id]')
     if (headers) {
       headers.forEach(element => {
         var title = element.innerText;


@carlinmack What do you want me to do with this code, leave it here or move it to the sholia.js file like the others?
Lines 125 to 134

sorry for not getting back to you on this, I reckon we should try externalise all of the JS (including these lines)

No problem. Ok, working on it!

carlinmack · 2021-08-18T13:28:48Z

Could you have function workWithQ(q, q2) {...} so that q2 can be accessed inside workWithQ? For #1552, I need to have consecutive ajax calls

$.ajax(endpointUrl, settings).then(function (data) {
    .... stuff with q ...
}).then(function (data) { 
   if (q2) {
       $.ajax(endpointUrl, settings).then(function (data) {
            .... stuff with q2 ...
       })
   }
})

and therefore need to have access to q2

curibe · 2021-08-19T16:35:45Z

Could you have function workWithQ(q, q2) {...} so that q2 can be accessed inside workWithQ? For #1552, I need to have consecutive ajax calls
$.ajax(endpointUrl, settings).then(function (data) {
    .... stuff with q ...
}).then(function (data) { 
   if (q2) {
       $.ajax(endpointUrl, settings).then(function (data) {
            .... stuff with q2 ...
       })
   }
})
and therefore need to have access to q2

@carlinmack sure, no problem. I can do it.

the js code is now implemented as a macro

Remove jinja expressions and pass some as arguments

Q2 was added as a argument of functions workWithQ, wikiItemAsDifferentAspect,wikiItemSubpage in case of its value could be required for later

curibe · 2021-08-26T06:09:10Z

Could you have function workWithQ(q, q2) {...} so that q2 can be accessed inside workWithQ? For #1552, I need to have consecutive ajax calls
$.ajax(endpointUrl, settings).then(function (data) {
    .... stuff with q ...
}).then(function (data) { 
   if (q2) {
       $.ajax(endpointUrl, settings).then(function (data) {
            .... stuff with q2 ...
       })
   }
})
and therefore need to have access to q2

@carlinmack I added the Q2 parameter to several functions in case of some of them could require it. This is because, according to the code above, it could be the functions wikiItemAsDifferentAspect or wikiItemSubpage. In any case, I added Q2 to these three functions: 384affe

fnielsen · 2021-10-25T16:19:22Z

Unfortunately there are now some issue that are had for me to resolve. I hope it would be possible to resolve this and add a new PR. I would like to see that we can get a good seperation in the code.

fnielsen · 2021-10-25T16:50:23Z

I will reopen and add a "needs rebasing" instead of closing it. It is unclear how much mess I have made by delaying this PR.

egonw · 2021-10-30T19:12:03Z

@curibe, I tried rebasing your 5 patches on top of the latest master, but there are more merge conflicts than I can resolve :( I can try harder, but it will take me a few hours, because it diverged quite a bit :(

curibe · 2021-11-01T21:32:16Z

Hi @egonw and @fnielsen . Ok, I'm going to take a look at it. Probably there were several changes in the js code since the PR was created, which makes the conflicts appear. Especially because the PR extracts the code out of the html.

codacy-production · 2022-12-16T10:59:26Z

scholia/app/static/scholia.js

+}
+
+function wikiItemSubpage(query, q, q2, urlfor) {
+    var endpointUrl = 'https://query.wikidata.org/sparql';


Codacy has a fix for the issue: Replace ·····var·endpointUrl·=·'https://query.wikidata.org/sparql' with ··var·endpointUrl·=·"https://query.wikidata.org/sparql"

Suggested change

var endpointUrl = 'https://query.wikidata.org/sparql';

var endpointUrl = "https://query.wikidata.org/sparql";

codacy-production · 2022-12-16T10:59:28Z

scholia/app/static/scholia.js

+					strictlanguage: true
+				}
+				return query
+			},


Codacy has a fix for the issue: Replace ························ with ············

Suggested change

},

},

codacy-production · 2022-12-16T10:59:29Z

scholia/app/static/scholia.js

+
+    var endpointUrl = 'https://query.wikidata.org/sparql';
+    settings = {
+    headers: { Accept: 'application/sparql-results+json' },


Codacy has a fix for the issue: Replace ·······headers:·{·Accept:·'application/sparql-results+json' with ····headers:·{·Accept:·"application/sparql-results+json"

Suggested change

headers: { Accept: 'application/sparql-results+json' },

headers: { Accept: "application/sparql-results+json" },

curibe changed the title ~~Separate js from html #785~~ [WIP] Separate js from html #785 Jul 22, 2021

curibe changed the title ~~[WIP] Separate js from html #785~~ Separate js from html #785 Jul 22, 2021

curibe mentioned this pull request Jul 29, 2021

Check that all SPARQL queries have been converted to externalized query format #1284

Closed

curibe linked an issue Jul 29, 2021 that may be closed by this pull request

JavaScript development setup #785

Open

curibe force-pushed the separate-js-from-html-785 branch from e9fb46f to 1a1ed42 Compare August 4, 2021 19:02

curibe requested review from carlinmack and fnielsen August 5, 2021 20:06

carlinmack suggested changes Aug 7, 2021

View reviewed changes

curibe requested a review from carlinmack August 9, 2021 19:37

curibe force-pushed the separate-js-from-html-785 branch from c95402d to e6ec48e Compare August 9, 2021 20:40

curibe commented Aug 9, 2021

View reviewed changes

curibe added 3 commits August 26, 2021 00:20

extract some js codes outside base.html

f0005c5

extract js code from 404_doi.html

a84316c

the js code is now implemented as a macro

fix: remove jinja expressions from scholia.js

31bceea

Remove jinja expressions and pass some as arguments

curibe force-pushed the separate-js-from-html-785 branch from e6ec48e to 31bceea Compare August 26, 2021 05:25

Add Q2 to several functions to use it later

384affe

Q2 was added as a argument of functions workWithQ, wikiItemAsDifferentAspect,wikiItemSubpage in case of its value could be required for later

Extract js code for anchor and table of content from base.html

433003e

fnielsen closed this Oct 25, 2021

fnielsen reopened this Oct 25, 2021

fnielsen added the needs rebasing label for pull requests that are sent back for rebasing on the latest master (by author or other) label Oct 25, 2021

egonw self-assigned this Oct 30, 2021

egonw assigned curibe and unassigned egonw Oct 30, 2021

Daniel-Mietchen added the final-report label Nov 15, 2021

Daniel-Mietchen removed the final-report label Dec 13, 2021

codacy-production bot reviewed Dec 16, 2022

View reviewed changes

	var endpointUrl = 'https://query.wikidata.org/sparql';
	var endpointUrl = "https://query.wikidata.org/sparql";

	headers: { Accept: 'application/sparql-results+json' },
	headers: { Accept: "application/sparql-results+json" },

Separate js from html #785 #1538

Are you sure you want to change the base?

Separate js from html #785 #1538

Uh oh!

Conversation

curibe commented Jul 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Separate JS codes from HTML

Uh oh!

carlinmack commented Jul 23, 2021

Uh oh!

curibe commented Jul 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlinmack commented Jul 26, 2021

Uh oh!

carlinmack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlinmack Aug 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlinmack Aug 7, 2021

Choose a reason for hiding this comment

Uh oh!

curibe Aug 9, 2021

Choose a reason for hiding this comment

Uh oh!

curibe Aug 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

curibe Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlinmack Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

curibe Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

carlinmack commented Aug 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

curibe commented Aug 19, 2021

Uh oh!

curibe commented Aug 26, 2021

Uh oh!

fnielsen commented Oct 25, 2021

Uh oh!

fnielsen commented Oct 25, 2021

Uh oh!

egonw commented Oct 30, 2021

Uh oh!

curibe commented Nov 1, 2021

Uh oh!

codacy-production bot Dec 16, 2022

Choose a reason for hiding this comment

Uh oh!

codacy-production bot Dec 16, 2022

Choose a reason for hiding this comment

Uh oh!

codacy-production bot Dec 16, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

curibe commented Jul 22, 2021 •

edited

Loading

curibe commented Jul 26, 2021 •

edited

Loading

carlinmack Aug 7, 2021 •

edited

Loading

curibe Aug 9, 2021 •

edited

Loading

carlinmack Aug 17, 2021 •

edited

Loading

carlinmack commented Aug 18, 2021 •

edited

Loading