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

Connects to #846. Connects to #885. #889

Merged
merged 14 commits into from
Aug 16, 2016
Merged

Conversation

jimmyzhen
Copy link
Contributor

This PR consists of implementations pertinent to the change requests in constructing the external ExAC linkout differently when there is no ExAC population data is found for the table display on the Population tab.

Technical details:

  1. In the hgvs_notation.js and parse-resources.js files, I added "if" statements to several places to ensure script execution even when the expected data is absent in the REST API response.
  2. In the basic_info.js file, removed no-longer-used code and refactored code to ensure script execution even when the expected data is absent in the REST API response.

Steps to test:

  1. Login and use 53981 and/or 90973 for selecting a variant (relevant to Wrapping long HGVS terms on Basic Info Tab #885).
  2. Expect to see that the long HGVS terms are ellipsis and not pushing the table beyond the page on the Basic Info tab.
  3. Also expect to see that the "asterisk-marked" canonical transcripts are no longer having a tinted red background color on the Basic Info tab (this was missed in my previous PR).
  4. The following variants are for testing the external ExAC linkout on the Population tab:
  • 55962 - expect the "See ExAC" link going to ExAC variant page.
  • 53905, 53205, 54032, 53981 - expect the "See ExAC" link going to ExAC region search.
  • CA501146, CA501097 - expect the "See ExAC" link going to ExAC homepage.

@kilodalton
Copy link
Contributor

@jimmyzhen Typically we have used a dotted line + hover text to show full HGVS names when they are long. Can you comment on why that has not been implemented here for long transcript names? There is currently no way to see the whole term(s).

(Basic info tab)
image

vs
(Variant selection modal)
image

@jimmyzhen
Copy link
Contributor Author

Hi @kilodalton,

When I worked on #812, I did consider implementing the hover text for long HGVS terms. There were 2 options I was considering:

  1. Hover text for all HGVS terms and neucleotide change transcripts on the Basic Info tab.
  2. Hover text only for long HGVS terms and neucleotide change transcripts on the Basic Info tab.

In option #1, I thought it would be undesirable for the hover text to be displayed for the short HGVS terms and transcripts in the tables.

In option #2, I thought it would be inconsistent for one variant to display hover text but not the other as an user may come to expect to see hover text consistently after seeing it in certain variants.

Finally, it felt incomplete to display hover text only for the neucleotide change transcripts but not the other data in the tables. And so, in the end, I opted for not displaying the hover text at all.

I am open to suggestions.

@jimmyzhen
Copy link
Contributor Author

Hi Karen,

I have committed the changes to this PR for displaying hover text and dotted underline on long HGVS terms whose text overflows. For a comparison to a short HGVS term, you may try 55847 vs 53981 and/or 90973.

@kilodalton kilodalton merged commit 0e87c2e into dev Aug 16, 2016
@kilodalton
Copy link
Contributor

Merged #846. Merged #885.

@kilodalton kilodalton deleted the 846_jz_exac_variant_link branch August 16, 2016 21:19
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

2 participants