Track tooltip shortDescription impossible to load #553

Closed
cmdcolin opened this Issue Jan 7, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@cmdcolin
Contributor

cmdcolin commented Jan 7, 2015

From mailing list "removing metadata.description HTML tags from the track list tooltips"

This thread shows that you can't set the track.shortDescription variable or the track.metadata.shortDescription variable to override the metadata.description being shown in the tooltip in the Hierarchical track selector.

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Jan 7, 2015

Contributor

When I started testing, I saw that the tootip is loaded as

track.shortDescription || track.description || track.Description || track.metadata && ( track.metadata.shortDescription || track.metadata.description ...

Therefore, I tried creating a track such as this:

  {
     "description": "testing",
     "key" : "myttrack"
  }

In this case, when debugging Hierarchical.js, the track.description will be undefined.

However, if you set

  {
     "metadata" { "description": "testing" },
     "key" : "myttrack"
  },

Then track.description will be set to "testing"

The reason for this is because this is using the "track" variable and not "trackConf" variable, and therefore, the metadata is merged into the "track" variable.

Another strange issue arises due to the merging behavior of the track variable

  {
     "metadata" { "description": "testing", "shortDescription": "short" },
     "key" : "myttrack"
  },

Now track.description="testing" but track.shortDescription is undefined...however track.shortdescription="short" is defined. Yes...it is lower case property name. Therefore, the code for getting the track.shortDescription or track.metadata.shortDescription from Hierarchical.js fails.

I believe the code should be reading from trackConf instead of track, because the trackConf variable is much more complete and mirrors the configuration file, where the track itself is sort of mangled.

Contributor

cmdcolin commented Jan 7, 2015

When I started testing, I saw that the tootip is loaded as

track.shortDescription || track.description || track.Description || track.metadata && ( track.metadata.shortDescription || track.metadata.description ...

Therefore, I tried creating a track such as this:

  {
     "description": "testing",
     "key" : "myttrack"
  }

In this case, when debugging Hierarchical.js, the track.description will be undefined.

However, if you set

  {
     "metadata" { "description": "testing" },
     "key" : "myttrack"
  },

Then track.description will be set to "testing"

The reason for this is because this is using the "track" variable and not "trackConf" variable, and therefore, the metadata is merged into the "track" variable.

Another strange issue arises due to the merging behavior of the track variable

  {
     "metadata" { "description": "testing", "shortDescription": "short" },
     "key" : "myttrack"
  },

Now track.description="testing" but track.shortDescription is undefined...however track.shortdescription="short" is defined. Yes...it is lower case property name. Therefore, the code for getting the track.shortDescription or track.metadata.shortDescription from Hierarchical.js fails.

I believe the code should be reading from trackConf instead of track, because the trackConf variable is much more complete and mirrors the configuration file, where the track itself is sort of mangled.

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Jan 7, 2015

Contributor

Note that the difference between track metadata and trackConf metadata is sort of interesting. For example, I previously wanted to be able to load the "category" for the hierarchical selector from external files like trackMetadata.csv, and I noticed that this category is loaded into the track variable, and not the trackConf variable. That's why I added this commit as it is: 78157b5

That sort of complicates the situation, but I think a suitable fix would mostly revolve around using trackConf instead of track, while perhaps falling back to track if we want to be able to load description from the external metadata

Contributor

cmdcolin commented Jan 7, 2015

Note that the difference between track metadata and trackConf metadata is sort of interesting. For example, I previously wanted to be able to load the "category" for the hierarchical selector from external files like trackMetadata.csv, and I noticed that this category is loaded into the track variable, and not the trackConf variable. That's why I added this commit as it is: 78157b5

That sort of complicates the situation, but I think a suitable fix would mostly revolve around using trackConf instead of track, while perhaps falling back to track if we want to be able to load description from the external metadata

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Jan 7, 2015

Contributor

This patch is a basic fix that doesn't allow loading from external metadata

diff --git a/src/JBrowse/View/TrackList/Hierarchical.js b/src/JBrowse/View/TrackList/Hierarchical.js
index 8a86206..fc1cb20 100644
--- a/src/JBrowse/View/TrackList/Hierarchical.js
+++ b/src/JBrowse/View/TrackList/Hierarchical.js
@@ -169,10 +169,16 @@ return declare(
             };

             category.pane.domNode.style.display = 'block';
+
+            // load tooltip, and if not a string, don't load tooltip
+            var mtitle= trackConf.shortDescription || trackConf.description ||  trackConf.Description ||
+                trackConf.metadata && (  trackConf.metadata.shortDescription ||  trackConf.metadata.description || trackConf.metadata.Description ) ||
+                trackConf.key || trackConf.label;
+
             var labelNode = dom.create(
                 'label', {
                     className: 'tracklist-label shown',
-                    title: Util.escapeHTML( track.shortDescription || track.description || track.Description || track.metadata && ( track.metadata.shortDescription || track.metadata.description || track.metadata.Description ) || track.key || trackConf.key || trackConf.label )
+                    title: lang.isString(mtitle)?Util.escapeHTML(mtitle):""
                 }, category.pane.containerNode );

It also says that if the description is not a string, then it is not loaded, because there are several instances in the volvox browser where it sets "description": 1. I'm not what this means but my code above, it means "don't load the description" in this case.

Contributor

cmdcolin commented Jan 7, 2015

This patch is a basic fix that doesn't allow loading from external metadata

diff --git a/src/JBrowse/View/TrackList/Hierarchical.js b/src/JBrowse/View/TrackList/Hierarchical.js
index 8a86206..fc1cb20 100644
--- a/src/JBrowse/View/TrackList/Hierarchical.js
+++ b/src/JBrowse/View/TrackList/Hierarchical.js
@@ -169,10 +169,16 @@ return declare(
             };

             category.pane.domNode.style.display = 'block';
+
+            // load tooltip, and if not a string, don't load tooltip
+            var mtitle= trackConf.shortDescription || trackConf.description ||  trackConf.Description ||
+                trackConf.metadata && (  trackConf.metadata.shortDescription ||  trackConf.metadata.description || trackConf.metadata.Description ) ||
+                trackConf.key || trackConf.label;
+
             var labelNode = dom.create(
                 'label', {
                     className: 'tracklist-label shown',
-                    title: Util.escapeHTML( track.shortDescription || track.description || track.Description || track.metadata && ( track.metadata.shortDescription || track.metadata.description || track.metadata.Description ) || track.key || trackConf.key || trackConf.label )
+                    title: lang.isString(mtitle)?Util.escapeHTML(mtitle):""
                 }, category.pane.containerNode );

It also says that if the description is not a string, then it is not loaded, because there are several instances in the volvox browser where it sets "description": 1. I'm not what this means but my code above, it means "don't load the description" in this case.

@rdhayes

This comment has been minimized.

Show comment
Hide comment
@rdhayes

rdhayes Jan 10, 2015

Contributor

Hi,

I see that working for us, thank you! We don't use the external metadata functionality.

Contributor

rdhayes commented Jan 10, 2015

Hi,

I see that working for us, thank you! We don't use the external metadata functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment