Skip to content
This repository has been archived by the owner on May 22, 2022. It is now read-only.

hide non-polymorphic sites feature added to the filter menu #188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bfrgoncalves
Copy link

Option to hide non-polymorphic sites. It hides the columns which nucleotide identity at a given position is above the threshold.

Bruno Gonçalves

@@ -25,6 +25,22 @@ const FilterMenu = MenuBuilder.extend({
return this.g.columns.set("hidden", hidden);
});

this.addNode("Hide non-polymorphic sites",(e) => {
var threshold = prompt("Enter identity threshold (in percent)", 100);
threshold = threshold / 100;
Copy link
Owner

Choose a reason for hiding this comment

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

it's very common to either check, or clamp user input. It could be as easy as this:

import {clamp} from "lodash"; // header
...
threshold = clamp(threshold / 100, 0, 1);

@wilzbach
Copy link
Owner

Option to hide non-polymorphic sites. It hides the columns which nucleotide identity at a given position is above the threshold.

Thanks. Looks all good to me 👍

I had some minor comments about the style though, do you mind addressing them?

@bfrgoncalves
Copy link
Author

Thanks for the comments for code improvement =) I reused your code from one of the filter options to create this new one but I will change the code and pull it here.

@wilzbach
Copy link
Owner

Thanks for the comments for code improvement =) I reused your code from one of the filter options to create this new one but I will change the code and pull it here.

Yep as said, some parts are still "dirty" as they were converted from CoffeeScript. Did you manage to adapt the changes? (I can't see them here).
If you ran into troubles, let me know ;-)

@wilzbach
Copy link
Owner

ping @bfrgoncalves ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants