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

Issue 634 v2 #1113

Merged
merged 4 commits into from
Mar 11, 2021
Merged
Changes from all commits
Commits
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
142 changes: 133 additions & 9 deletions resource/js/hierarchy.js
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -288,17 +288,18 @@ function getParams(node) {
return $.param({'uri' : nodeId, 'lang' : clang});
}

function pickLabelFromScheme(scheme) {
function pickLabel(entity) {
var label = '';
if (scheme.prefLabel)
label = scheme.prefLabel;
else if (scheme.label)
label = scheme.label;
else if (scheme.title)
label = scheme.title;
if (entity.prefLabel)
label = entity.prefLabel;
else if (entity.label)
label = entity.label;
else if (entity.title)
label = entity.title;
return label;
}

/*
Copy link
Member

Choose a reason for hiding this comment

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

It is not good practice to keep old code in a comment (that's what version control is for!), could you please remove this?

function schemeRoot(schemes) {
var topArray = [];
for (var i = 0; i < schemes.length; i++) {
Expand All @@ -318,6 +319,101 @@ function schemeRoot(schemes) {
}
return topArray;
}
*/

function schemeRoot(schemes) {
var topArray = [];

// Step 1 : gather domain list
var domains=[];

for (var i = 0; i < schemes.length; i++) {
// iterate on schemes subjects...
if(schemes[i].subject != null) {
var schemeDomain = schemes[i].subject.uri;

// test if domain was already found
var found = false;
for (var k = 0; k < domains.length; k++) {
if(domains[k].uri===schemeDomain){
found = true;
break;
}
}

// if not found, store it in domain list
if(!found) {
domains.push(schemes[i].subject);
}
}
}

// Step 2 : create tree nodes for each domain
for (var i = 0; i < domains.length; i++) {
var theDomain = domains[i];
var theDomainLabel = pickLabel(theDomain);

// avoid creating entries with empty labels
if(theDomainLabel != '') {
// Step 2.1 : create domain node without children
var domainObject = {
text: theDomainLabel,
// note that the class 'domain' will make sure the node will be sorted _before_ others
// (see the 'sort' function at the end)
a_attr : { "href" : vocab + '/' + lang + '/page/?uri=' + theDomain.uri, 'class': 'domain'},
uri: theDomain.uri,
children: [],
state: { opened: false }
};

// Step 2.2 : find the concept schemes in this domain and add them as children
for (var k = 0; k < schemes.length; k++) {
var theScheme = schemes[k];
var theSchemeLabel = pickLabel(theScheme);

// avoid creating entries with empty labels
if(theSchemeLabel != '') {
if((theScheme.subject) != null && (theScheme.subject.uri===theDomain.uri)) {
domainObject.children.push(
{
text: theSchemeLabel,
a_attr:{ "href" : vocab + '/' + lang + '/page/?uri=' + theScheme.uri, 'class': 'scheme'},
uri: theScheme.uri,
children: true,
state: { opened: false }
}
);
}
}
} // end iterating on schemes

topArray.push(domainObject);
}
} // end iterating on domains

// Step 3 : add the schemes without any subjects after the subjects node
for (var k = 0; k < schemes.length; k++) {
var theScheme = schemes[k];

if(theScheme.subject == null) {
// avoid creating entries with empty labels
var theSchemeLabel = pickLabel(theScheme);
if(theSchemeLabel != '') {
topArray.push(
{
text:theSchemeLabel,
a_attr:{ "href" : vocab + '/' + lang + '/page/?uri=' + theScheme.uri, 'class': 'scheme'},
uri: theScheme.uri,
children: true,
state: { opened: false }
}
);
}
}
}

return topArray;
}

function addConceptsToScheme(topConcept, childObject, schemes) {
for (var j in schemes) {
Expand Down Expand Up @@ -440,6 +536,7 @@ function getTreeConfiguration() {
var aNode = this.get_node(a);
var bNode = this.get_node(b);

// sort on notation if requested
if (window.showNotation) {
var aNotation = aNode.original.notation;
var bNotation = bNode.original.notation;
Expand All @@ -456,8 +553,35 @@ function getTreeConfiguration() {
else return -1;
}
else if (bNotation) return 1;
}
return naturalCompare(aNode.text.toLowerCase(), bNode.text.toLowerCase());
} else {
Copy link
Member

@osma osma Mar 10, 2021

Choose a reason for hiding this comment

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

This seems a bit overly complex. I tested replacing the whole else block with this:

        } else {
          // no sorting on notation requested
          // make sure the tree nodes with class 'domain' are sorted before the others
          // aDomain/bDomain will be 0 if a/b has a domain class, else 1
          var aDomain = (int) (aNode.original.a_attr['class'] != 'domain');
          var bDomain = (int) (bNode.original.a_attr['class'] != 'domain');
          return naturalCompare(aDomain + " " + aNode.text.toLowerCase(),
                                bDomain + " " + bNode.text.toLowerCase());
        }

It gave me the same result.

Copy link
Collaborator

@kinow kinow Mar 10, 2021

Choose a reason for hiding this comment

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

Didn't you get any error due to the (int) casting? I normally use parseInt() or Number in JS/TS, and think that's still not available in ES6. But tested in a codepen pen just to make sure. Maybe the test data doesn't include what's necessary to go into this else block?

image

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry, my bad! You are of course right @kinow, I was mixing up Java syntax with JS. Just shows how little I do JS coding :) And indeed, showNotation was enabled because that is the default setting for vocabularies, so execution never went to that else block.

Here is a better fix that I also tested, double-checking that it does get executed (I also checked that if I invert the logic here, the domains will be sorted last):

        } else {
          // no sorting on notation requested
          // make sure the tree nodes with class 'domain' are sorted before the others
          // aDomain/bDomain will be "0" if a/b has a domain class, else "1"
          var aDomain = (aNode.original.a_attr['class'] == 'domain') ? "0" : "1";
          var bDomain = (bNode.original.a_attr['class'] == 'domain') ? "0" : "1";
          return naturalCompare(aDomain + " " + aNode.text.toLowerCase(),
                                bDomain + " " + bNode.text.toLowerCase());
        }

// no sorting on notation requested
// make sure the tree nodes with class 'domain' are sorted before the others
var aClass = aNode.original.a_attr['class'];
var bClass = bNode.original.a_attr['class'];

if(aClass) {
if(bClass) {
if(aClass == 'domain' && bClass == 'domain') {
// 2 domains : alpha sort
return naturalCompare(this.get_text(a).toLowerCase(), this.get_text(b).toLowerCase());
}
else if(aClass == 'domain' && bClass != 'domain') {
// A is before B
return -1;
}
else if(aClass != 'domain' && bClass == 'domain') {
// B is before A
return 1;
}
} else {
// A has class, but not B (should not happen) : alpha sort
return naturalCompare(this.get_text(a).toLowerCase(), this.get_text(b).toLowerCase());
}
} else {
// A has no class (should not happen) : alpha sort
return naturalCompare(this.get_text(a).toLowerCase(), this.get_text(b).toLowerCase());
}
}
}
});
}
Expand Down