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

Add show/hide deleted/hidden links when history is filtered... #4581

Merged
merged 4 commits into from Sep 12, 2017

Conversation

Projects
None yet
4 participants
@chambm
Copy link
Contributor

commented Sep 8, 2017

by the find box; addresses #4103

I'm totally naive about editing the client (including adding a test to check this), but it works AFAICT...

@galaxybot galaxybot added the triage label Sep 8, 2017

@galaxybot galaxybot added this to the 17.09 milestone Sep 8, 2017

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Sep 11, 2017

@galaxybot test this

@chambm

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2017

Also, the 'show deleted' and 'show hidden' language tokens are probably new (and unlocalized). I didn't think I could get counts of hidden and deleted files also matching the find criteria. But I didn't try, so maybe it would have worked.

@dannon

This comment has been minimized.

Copy link
Member

commented Sep 11, 2017

@chambm Thanks for this, checking it out now.

One quick note though is that you will need to make the client as well - make client from the galaxy root folder, and commit those changes. I PR'd what that'd look like here: chambm#2, which you can merge, or you're welcome to close that and do it yourself.

@dannon

This comment has been minimized.

Copy link
Member

commented Sep 11, 2017

Still tinkering with it, but one minor defect here is that [No matching datasets found] is no longer rendered when clicking 'show hidden', etc., when there are no results.

@chambm

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2017

I confess to not understanding where that message box comes from in the old code. It seems to come from the base template in history-view.js, but I don't understand how my changes prevent that from showing (but then I also don't understand why the old code was allowing to show either).

@dannon

This comment has been minimized.

Copy link
Member

commented Sep 11, 2017

@chambm I'll take a look and see what I can figure out

@chambm

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2017

Did you find out what caused the no matches infobox to disappear?

@dannon

This comment has been minimized.

Copy link
Member

commented Sep 12, 2017

I made a lot of progress, but no solution yet. I was about to merge this and bugfix that bit after the fact, so we don't hold up the release freeze (with the issue being purely cosmetic, and minor).

Basically, it looks like model attributes aren't being recognized, so the check to see "Do we know what we need to know to say there are no results?" fails incorrectly.

@chambm

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2017

Yeah I do think it's a very minor bug because you get the "Found 0" caption in any case. Thanks!

@dannon dannon merged commit fd19267 into galaxyproject:dev Sep 12, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: JavaScript No alert changes
Details
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.