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

Study View: Always show selected genes on top and alternate selected rows color #2980

Merged
merged 1 commit into from
Jan 22, 2020

Conversation

kalletlak
Copy link
Member

Fix cBioPortal/cbioportal#6896

Fixes

  • Always show selected genes on top of table(selected rows will be ignored from sorting)
  • Alternate selected rows color

Screen Shot 2020-01-13 at 2 03 28 PM

Copy link
Member

@jjgao jjgao left a comment

Choose a reason for hiding this comment

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

Looks great from product perspective 👍 Thanks, @kalletlak

if (index === -1) {
return null;
}
return index % 2 === 0 ? styles.highlightedEvenRow : styles.highlightedOddRow;
Copy link
Contributor

@adamabeshouse adamabeshouse Jan 15, 2020

Choose a reason for hiding this comment

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

you don't need to do this with code, you can do it with CSS, for eample in simpleTable/styles.scss we do:

tbody>tr:nth-of-type(odd).highlighted {
    background-color: #9FAFD1 !important;
  }

  tbody>tr:nth-of-type(even) { // solid background - hard coded hack, react-bootstrap .table-striped makes ODD # rows grey
    background-color: rgb(255,255,255);
  }

  tbody>tr:nth-of-type(even).highlighted {
    background-color: #B0BED9;
  }

I think we should do it like that because it's more efficient and simpler in the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here the color is not alternating. alternating color for each selection (there can be more than one row in each selection)

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhh, I see

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a chance of many selected rows? if so then this process will be slow - it would be faster to have a map that takes uniquekey to the first index of filter in which it appears, so that you can check this in constant time

Copy link
Contributor

@adamabeshouse adamabeshouse left a comment

Choose a reason for hiding this comment

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

Looks good, please see my comments for a few requested changes.

@kalletlak kalletlak force-pushed the fix-6896 branch 2 times, most recently from 3adbda1 to ea09d84 Compare January 16, 2020 19:05
@kalletlak kalletlak force-pushed the fix-6896 branch 2 times, most recently from 216cf2d to 107dca1 Compare January 17, 2020 20:36
@kalletlak kalletlak force-pushed the fix-6896 branch 2 times, most recently from 84fa0d8 to d4b4ba6 Compare January 22, 2020 17:05
@alisman alisman merged commit f29d0e3 into cBioPortal:master Jan 22, 2020
@kalletlak kalletlak deleted the fix-6896 branch July 8, 2024 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

on study view move selections to the top of the table
4 participants