iobio / gene.iobio

Gene.iobio vue
MIT License
55 stars 11 forks source link

Added selectVariantAnnotationDialog #846 #1001

Closed YangQi007 closed 11 months ago

YangQi007 commented 1 year ago

Added selectVariantAnnotationDialog, the mosaic option is not completed yet.

tonydisera commented 1 year ago

Hi @YangQi007. I've left comments on issue #846.

tonydisera commented 11 months ago

Hi @YangQi007. I pulled down your branch and tested this morning. There are some fixes I requested in the last pull request that are not yet fixed. Here are screen prints with comments.

Screenshot 2023-10-07 at 9 33 44 AM Screenshot 2023-10-07 at 9 28 58 AM

Screenshot 2023-10-07 at 9 05 38 AM

Screenshot 2023-10-07 at 9 02 48 AM
tonydisera commented 11 months ago

See comments above ^

YangQi007 commented 11 months ago

Hi, @tonydisera I fixed the user interface based on your comments. Here, there is a thing I would like to bring up to your attention. When parsing the vcf file, there are abbreviations for annotations, but not for calling the mosaic API. So on the user interface, the UI is not consistent.

Screenshot 2023-10-10 at 9 19 38 AM
tonydisera commented 11 months ago

@YangQi007, I like the way the interface looks with the fixed-width columns. It is okay that the Mosaic variant annotations look a bit different.

tonydisera commented 11 months ago

@YangQi007, there is a problem with the tooltip on the annotations in VariantInspectCard. When I hover over the annotation abbreviation, the screen flickers and the tooltip shows at the bottom of the panel rather than by the label I am hovering over.

Screenshot 2023-10-10 at 11 36 28 AM
tonydisera commented 11 months ago

@YangQi007 , please change the "select" button to have the following styling. This will align with the header.

Screenshot 2023-10-10 at 11 43 41 AM

And the icon within the button should have font-size: 22px

tonydisera commented 11 months ago

@YangQi007, we can probably have a narrower fixed width so that the values are closer to the labels.

Screenshot 2023-10-10 at 11 39 16 AM
tonydisera commented 11 months ago

On the "select annotations" dialog, there are still a few format issues to address. Also, it would be nice to have a "select all" checkbox so that the user doesn't have to click on every annotation.

Screenshot 2023-10-10 at 11 47 09 AM
tonydisera commented 11 months ago

Also, let's remove 'custom' from 'Custom annotations'. I think this will look better.

Screenshot 2023-10-10 at 12 02 26 PM
YangQi007 commented 11 months ago

@YangQi007, we can probably have a narrower fixed width so that the values are closer to the labels. Screenshot 2023-10-10 at 11 39 16 AM

As to your comment about narrowing down the fixed width. I did that because in mosaic the name of annotations are longer. So if make the width smaller, the format of name of annotations will be not consistent.

Screenshot 2023-10-10 at 12 09 12 PM
tonydisera commented 11 months ago

@YangQi007, we can probably have a narrower fixed width so that the values are closer to the labels. Screenshot 2023-10-10 at 11 39 16 AM

As to your comment about narrowing down the fixed width. I did that because in mosaic the name of annotations are longer. So if make the width smaller, the format of name of annotations will be not consistent. Screenshot 2023-10-10 at 12 09 12 PM

Good point, @YangQi007. Let's leave the fixed width to your proposed width.

tonydisera commented 11 months ago

Hi @YangQi007 - I just pulled down your branch and tested. Some of the items haven't been fixed yet. Have you pushed all of your changes? I don't see the "Select all" checkbox on the 'Select annotations' dialog and the number of annotations next to the header. The tooltip still flickers on the VariantInspectCard when I hover over the icon. Also the tooltip isn't placed correctly.

YangQi007 commented 11 months ago

Hi @YangQi007 - I just pulled down your branch and tested. Some of the items haven't been fixed yet. Have you pushed all of your changes? I don't see the "Select all" checkbox on the 'Select annotations' dialog and the number of annotations next to the header. The tooltip still flickers on the VariantInspectCard when I hover over the icon. Also the tooltip isn't placed correctly.

You got email for PR? I didn't commit them yet. I just fixed the tooltip flicking and changed their position. I am still working on selectAll feature.

tonydisera commented 11 months ago

I see. The 'review requested' (above) must happen when a push to your branch is made.

YangQi007 commented 11 months ago

I see. The 'review requested' (above) must happen when a push to your branch is made.

Hi, @tonydisera just pushed the changes, now it should be all there. Please check it out.

YangQi007 commented 11 months ago

@tonydisera I didn't add a new checkbox for the deselect option because the 'Cancel' button has the same functionality.

tonydisera commented 11 months ago

@YangQi007 This looks great. These last changes make the UI look really polished. I am merging this pull request.