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

Improve keys formatting in "About this track" #1148

Merged
merged 2 commits into from Aug 1, 2018

Conversation

Projects
None yet
3 participants
@wbazant
Copy link
Contributor

wbazant commented Jul 31, 2018

Changes:

  • use this.config.metadata if we can, trackMetaDataStore mangles the keys
  • if trackSelector.renameFacets was defined, use the formatted keys provided
  • instead of blanket ucFirst: only apply if all lowercase, and also swap underscores for spaces to show camel case nicer
  • sort keys alphabetically (instead of current random order)

Why:
I'm setting up a fairly large and heterogeneous tracks display. An example URL on a perennial and unstable test site that nobody should rely on, but currently shows a vaguely correct picture is:
http://test.parasite.wormbase.org/jbrowse/index.html?data=%2Fjbrowse-data%2Fbursaphelenchus_xylophilus_prjea64437%2Fdata
I am aggregating data into faceted track displays. They work fantastically - I believe it is genuinely possible to use the display to find relevant tracks there among thousands.

I can't fit all the metadata I have into facets, so I pick the important ones, and want the user to access the rest in "About this track". Unfortunately some of the data there comes out mangled. I found that it happens in this code here, and changed it. I am aware of fmtDetailField_*, but I think this patch will be better, and I think it will also work for others.

Improve keys formatting in "About this track"
- use this.config.metadata if we can, trackMetaDataStore mangles the keys
- if trackSelector.renameFacets was defined, use the formatted keys provided
- instead of blanket ucFirst: only apply if all lowercase, and also swap underscores for spaces to show camel case nicer
- sort keys alphabetically (instead of current random order)
if (this.browser && this.browser.config && this.browser.config.trackSelector && this.browser.config.trackSelector.renameFacets){
var metadataCopy = {};
for (var k in metadata){
key = this.browser.config.trackSelector.renameFacets[k] || k;

This comment has been minimized.

@rbuels

rbuels Jul 31, 2018

Collaborator

key isn't defined, which is why the build is not completing. could you change that line to let key = ...?

@wbazant

This comment has been minimized.

Copy link
Contributor Author

wbazant commented Jul 31, 2018

BTW I had doubts about trying to improve camel case keys with

str.replace(/_/g, " ")

after I realised it's not backwards compatible - it might break somebody's fmtDetailField_* if they named a keys in camel case, and also set a fmtDetailField_$key.

Less ambitious behaviour of "ucfirst if the word appears to be all lowercase" instead of "ucfirst everything" is unlikely to break keys that are not in lowercase, and has almost all of the benefit: the code doesn't destroy information any more, and any camel case properties can be renamed in the trackSelector.renameFacets.

Actually I only made my keys camel_case because documentation about faceted track selector insisted on the keys being in lowercase and I didn't know why. Maybe they have to be lowercase, but spaces are okay? What format is actually suggested for them?

@rbuels

rbuels approved these changes Jul 31, 2018

@rbuels rbuels requested a review from cmdcolin Aug 1, 2018

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 1, 2018

This is a really tangential comment but sort of referred to by @wbazant which is sort of a fact that the facetted grid is set to screen width and it might be cool if it could be wider than screen limit for people with so much metadata to show

@wbazant

This comment has been minimized.

Copy link
Contributor Author

wbazant commented Aug 1, 2018

@cmdcolin I have tried removing the width property on a large grid. It did something reasonable: the elements were 6 em in size and I could scroll left/right conveniently. I still had very many columns and sparse data, so it was hard to find something there, but it's easy to support and could be a reasonable option for fairly full grids that are beginning to be too large. For my large grid, I wish it could have flexible column display, showing only columns with data, so that it's easy to look at its subset. See issue #1150.

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 1, 2018

This looks pretty good IMO.

It does look like your trackList.json is very large though, almost 9 MB. Might be all that metadata? Or just many many tracks...in any case you might want gzip enabled on your servers :)

I guess it might be interesting if the trackMetadata didn't mangle the keys (presumably, mostly by lower casing them) but I think this PR is good to go

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 1, 2018

I will add some sample random metadata to a track on volvox so we can just see the rich metadata in the about this track page

@cmdcolin cmdcolin merged commit 8284e5a into GMOD:dev Aug 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rbuels rbuels added this to the 1.15.1 milestone Aug 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.