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

Fixes PTV-225 #1169

Merged
merged 3 commits into from
Sep 28, 2017
Merged

Fixes PTV-225 #1169

merged 3 commits into from
Sep 28, 2017

Conversation

thomasoniii
Copy link
Contributor

This fixes sorting by type, with a caveat -

The type will sort by the fully qualified name, which may be a little confusing to the user. e.g., in the collapsed view of the data, the user may see "GenomeAnnotation" and "Media" and expect "GenomeAnnotation" to be first in the sorted list. It won't be.

That's because the actual types are "KBaseGenomeAnnotations.GenomeAnnotation" and "KBaseBioChem.Media", and will in turn sort KBaseBioChem.Media first.

I left the behavior as is, since the user could expand the data item and see the full name, but we may want to change this.

@briehl
Copy link
Member

briehl commented Sep 21, 2017

I think this is a good start, but since the collapsed view is mostly what users see, we should probably sort by that and not the fully qualified name.

Then again, I'm not sure what the best answer is. @sychan? @nlharris?

@briehl
Copy link
Member

briehl commented Sep 21, 2017

I've thought about this more, and it makes more sense to filter by the shortened name. Other places in that interface (the filter dropdown) just make use of the shortened name, and we should be consistent. Also, the shorter terms like "Media" are the first things that users will see, and I suspect the only things for many users.

@nlharris
Copy link
Contributor

I agree. Users will be confused if the sort is by something they don't see. Use the shortened ("pretty") names for sorting.

@thomasoniii
Copy link
Contributor Author

Updated so it'll sort on the non-qualified name.

@@ -1492,9 +1492,11 @@ define([
.append('type')
.on('click', function () {
self.sortData(function (a, b) {
if (self.dataObjects[a].info[2].toUpperCase() > self.dataObjects[b].info[2].toUpperCase())
let aType = self.dataObjects[a.objId].info[2].toUpperCase().match(/\.(.+)/)[1];
Copy link
Member

Choose a reason for hiding this comment

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

Sadly, we're still in ES5 (and there's no Babel installed on the build path yet...), so "let" is what's causing the build to fail.

Copy link
Member

Choose a reason for hiding this comment

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

That took me entirely too long to track down. Sorry!

@briehl briehl merged commit fbb9a93 into develop Sep 28, 2017
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.

None yet

3 participants