-
Notifications
You must be signed in to change notification settings - Fork 45
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
Hide readviz by default #1556
Hide readviz by default #1556
Conversation
97a53e7
to
9953610
Compare
9953610
to
42a8a5a
Compare
@@ -307,8 +325,8 @@ class ReadData extends Component<ReadDataProps, ReadDataState> { | |||
} | |||
|
|||
loadInitialTracks() { | |||
;['exome', 'genome'].forEach((exomeOrGenome) => { | |||
;['het', 'hom', 'hemi'].forEach((category) => { | |||
;(['exome', 'genome'] as SequencingType[]).forEach((exomeOrGenome) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this works, it'd be more idiomatic to write ['exome', 'genome'].forEach((exomeOrGenome: SequencingType) => ...
. I think TS should be able to infer that properly. Same deal in the next line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have liked that better too, but unfortunately TS complains about doing it that way, as the forEach
is being called on an array of strings.
browser/src/ReadData/ReadData.tsx
Outdated
<CheckboxInput | ||
id="always-load" | ||
checked={alwaysLoad} | ||
onChange={(e: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be a type you can use for e
here, MouseEvent
or something similar IIRC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cut, print, that's a wrap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me from a UI / Text copy perspective
d43f30a
to
e8d03e6
Compare
db93cca
to
a92857d
Compare
Resolves #633
Hides readviz by default on Variant pages to try and save on egress fees.
Gives two options,
localStorage
to save the value of this on load and when toggling thisAlso adds tracking to these two buttons to try and see how often people use these