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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified ...remote/screenshots/reference/filter_study_from_url_element_chrome_1600x1000.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...hots/reference/fusion_filter_filter_study_from_url_element_chrome_1600x1000.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/pages/studyView/StudyViewPageStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ export class StudyViewPageStore {
}

public getGeneFiltersByUniqueKey(uniqueKey: string) {
return _.flatMapDeep(toJS(this._geneFilterSet.get(uniqueKey)) || []);
return toJS(this._geneFilterSet.get(uniqueKey)) || [];
}

public getClinicalDataFiltersByUniqueKey(uniqueKey: string) {
Expand Down
15 changes: 0 additions & 15 deletions src/pages/studyView/TableUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {getFrequencyStr, getCNAByAlteration} from "pages/studyView/StudyViewUtil
import {GenePanelList} from "pages/studyView/table/StudyViewGenePanelModal";
import {CSSProperties} from "react";
import * as _ from "lodash";
import {GeneTableUserSelectionWithIndex} from "pages/studyView/table/GeneTable";

export type AlteredGenesTableUserSelectionWithIndex = {
entrezGeneId: number;
Expand Down Expand Up @@ -88,17 +87,3 @@ export function getFreqColumnRender(type: FreqColumnType, numberOfProfiledCases:
</DefaultTooltip>
);
}

export function rowIsChecked(uniqueKey: string, preSelectedRows: GeneTableUserSelectionWithIndex[], selectedRows: GeneTableUserSelectionWithIndex[]) {
return _.some(
preSelectedRows.concat(selectedRows),
(row: GeneTableUserSelectionWithIndex) => row.uniqueKey === uniqueKey
);
};

export function rowIsDisabled(uniqueKey: string, selectedRows: GeneTableUserSelectionWithIndex[]) {
return _.some(
selectedRows,
(row: GeneTableUserSelectionWithIndex) => row.uniqueKey === uniqueKey
);
};
54 changes: 47 additions & 7 deletions src/pages/studyView/table/FixedHeaderTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from "react";
import {Column, LazyMobXTableStore, SortDirection} from "../../../shared/components/lazyMobXTable/LazyMobXTable";
import {Column, LazyMobXTableStore, SortDirection, lazyMobXTableSort} from "../../../shared/components/lazyMobXTable/LazyMobXTable";
import {
Column as RVColumn,
SortDirection as RVSortDirection,
Expand All @@ -17,9 +17,11 @@ import {If} from 'react-if';
import autobind from 'autobind-decorator';
import {inputBoxChangeTimeoutEvent} from "../../../shared/lib/EventUtils";
import {DefaultTooltip} from "cbioportal-frontend-commons";
import { SimpleGetterLazyMobXTableApplicationDataStore } from "shared/lib/ILazyMobXTableApplicationDataStore";

export type IFixedHeaderTableProps<T> = {
columns: Column<T>[],
fixedTopRowsData?: T[];
data: T[];
sortBy?: string;
sortDirection?: SortDirection;
Expand All @@ -38,6 +40,7 @@ export type IFixedHeaderTableProps<T> = {
removeAllDisabled?:boolean;
showSelectableNumber?: boolean;
isSelectedRow?: (data: T) => boolean;
highlightedRowClassName?: (data: T) => string;
autoFocusSearchAfterRendering?:boolean;
afterSorting?: (sortBy: string, sortDirection: SortDirection) => void;
};
Expand All @@ -47,6 +50,24 @@ const RVSDTtoStrType = {
['asc' as SortDirection]: RVSortDirection.ASC
};

export class FixedHeaderTableDataStore extends SimpleGetterLazyMobXTableApplicationDataStore<any> {

constructor(getData: () => any[], fixedTopRowsData: any[]) {
super(getData);
this.fixedTopRowsData = fixedTopRowsData;
}

private fixedTopRowsData: any[];

@computed get sortedData() {
// if not defined, use default values for sortMetric and sortAscending
const sortMetric = this.sortMetric || (() => 0);
const sortAscending = this.sortAscending !== undefined ? this.sortAscending : true;

return [...this.fixedTopRowsData, ...lazyMobXTableSort(this.allData, sortMetric, sortAscending)];
}
}

@observer
export default class FixedHeaderTable<T> extends React.Component<IFixedHeaderTableProps<T>, {}> {
private _store: LazyMobXTableStore<T>;
Expand Down Expand Up @@ -78,32 +99,51 @@ export default class FixedHeaderTable<T> extends React.Component<IFixedHeaderTab
this.initDataStore();
}

componentWillReceiveProps(nextProps: any) {
componentWillReceiveProps(nextProps: IFixedHeaderTableProps<T>) {
this.updateDataStore(nextProps);
}

updateDataStore(nextProps: any) {
updateDataStore(nextProps: IFixedHeaderTableProps<T>) {
const tableDataStore = new FixedHeaderTableDataStore(
() => {
return this.props.data;
},
this.props.fixedTopRowsData || []
);
this._store.setProps({
columns: nextProps.columns,
data: nextProps.data,
dataStore: tableDataStore,
initialSortColumn: this._sortBy,
initialSortDirection: this._sortDirection
});
}

initDataStore() {
const tableDataStore = new FixedHeaderTableDataStore(
() => {
return this.props.data;
},
this.props.fixedTopRowsData || []
);
this._store = new LazyMobXTableStore<T>({
columns: this.props.columns,
data: this.props.data,
dataStore: tableDataStore,
initialSortColumn: this._sortBy,
initialSortDirection: this._sortDirection
});
}

@autobind
rowClassName({index}: any) {
rowClassName({ index }: any) {
if (index > -1 && this.isSelectedRow(this._store.dataStore.sortedFilteredData[index])) {
return classnames(styles.row, styles.highlightedRow);
const classNames: string[] = [styles.row]
if (this.props.highlightedRowClassName) {
const className = this.props.highlightedRowClassName(this._store.dataStore.sortedFilteredData[index]);
classNames.push(className);
} else {
classNames.push(styles.highlightedRow);
adamabeshouse marked this conversation as resolved.
Show resolved Hide resolved
}
return classnames(classNames);
} else if (index < 0) {
return styles.headerRow;
} else {
Expand Down
136 changes: 66 additions & 70 deletions src/pages/studyView/table/GeneTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@ import {
import {
OncokbCancerGene
} from "pages/studyView/StudyViewPageStore";
import {getFreqColumnRender, getGeneColumnHeaderRender, rowIsChecked, rowIsDisabled} from "pages/studyView/TableUtils";
import {getFreqColumnRender, getGeneColumnHeaderRender} from "pages/studyView/TableUtils";
import {GeneCell} from "pages/studyView/table/GeneCell";
import LabeledCheckbox from "shared/components/labeledCheckbox/LabeledCheckbox";
import styles from "pages/studyView/table/tables.module.scss";
import MobxPromise from "mobxpromise";

export type GeneTableUserSelectionWithIndex = {
uniqueKey: string;
rowIndex: number;
};
import { stringListToIndexSet, stringListToSet } from "cbioportal-frontend-commons";
import ifNotDefined from "shared/lib/ifNotDefined";

export type GeneTableRow = OncokbCancerGene & {
entrezGeneId: number
Expand Down Expand Up @@ -61,7 +58,7 @@ export type GeneTableProps = & {
promise: MobxPromise<GeneTableRow[]>;
width: number;
height: number;
filters: string[];
filters: string[][];
onUserSelection: (value: string[]) => void;
numOfSelectedSamples: number;
onGeneSelect: (hugoGeneSymbol: string) => void;
Expand Down Expand Up @@ -89,7 +86,7 @@ class GeneTableComponent extends FixedHeaderTable<GeneTableRow> {

@observer
export class GeneTable extends React.Component<GeneTableProps, {}> {
@observable protected preSelectedRows: GeneTableUserSelectionWithIndex[] = [];
@observable protected selectedRowsKeys: string[] = [];
@observable protected sortBy: GeneTableColumnKey;
@observable private sortDirection: SortDirection;
@observable private modalSettings: {
Expand Down Expand Up @@ -367,12 +364,39 @@ export class GeneTable extends React.Component<GeneTableProps, {}> {
return this.isFilteredByCancerGeneList ? _.filter(this.props.promise.result, data => data.isCancerGene) : (this.props.promise.result || []);
}

@computed get flattenedFilters() {
return _.flatMap(this.props.filters);
}

@computed get selectableTableData() {
if (this.flattenedFilters.length === 0) {
return this.tableData;
}
return _.filter(this.tableData, data => !this.flattenedFilters.includes(data.uniqueKey));
}

@computed
get preSelectedRows() {
if (this.flattenedFilters.length === 0) {
return [];
}
const order = stringListToIndexSet(this.flattenedFilters);
return _.chain(this.tableData)
.filter(data => this.flattenedFilters.includes(data.uniqueKey))
.sortBy<GeneTableRow>(data => ifNotDefined(order[data.uniqueKey], Number.POSITIVE_INFINITY))
.value();
}

@computed
get preSelectedRowsKeys() {
return this.preSelectedRows.map(row => row.uniqueKey);
}

@computed
get tableColumns() {
return this.props.columns.map(column => this.getDefaultColumnDefinition(column.columnKey, this.columnsWidth[column.columnKey], this.cellMargin[column.columnKey]));
}


@autobind
@action
toggleModal(panelName: string) {
Expand All @@ -399,88 +423,58 @@ export class GeneTable extends React.Component<GeneTableProps, {}> {
return !!this.props.cancerGeneFilterEnabled && this.props.filterByCancerGenes;
}

@computed get allSelectedRowsKeysSet() {
return stringListToSet([...this.selectedRowsKeys, ...this.preSelectedRowsKeys]);
}

@autobind
isChecked(uniqueKey: string) {
return rowIsChecked(uniqueKey, this.preSelectedRows, this.selectedRows);
return !!this.allSelectedRowsKeysSet[uniqueKey];
}

@autobind
isDisabled(uniqueKey: string) {
return rowIsDisabled(uniqueKey, this.selectedRows);
return _.some(this.preSelectedRowsKeys, (key) => key === uniqueKey);
}

@autobind
togglePreSelectRow(uniqueKey: string) {
const record: GeneTableUserSelectionWithIndex | undefined = _.find(
this.preSelectedRows,
(row: GeneTableUserSelectionWithIndex) => row.uniqueKey === uniqueKey
);
const record = _.find(this.selectedRowsKeys,(key) => key === uniqueKey);
if (_.isUndefined(record)) {
let dataIndex = -1;
// definitely there is a match
const datum = _.find(
this.tableData,
(row, index: number) => {
const exist = row.uniqueKey! === uniqueKey;
if (exist) {
dataIndex = index;
}
return exist;
}
);

if (!_.isUndefined(datum)) {
this.preSelectedRows.push({
rowIndex: dataIndex,
uniqueKey: datum.uniqueKey!,
});
}
this.selectedRowsKeys.push(uniqueKey);
} else {
this.preSelectedRows = _.xorBy(this.preSelectedRows, [record], "rowIndex");
this.selectedRowsKeys = _.xorBy(this.selectedRowsKeys, [record]);
}
}

@autobind
@action
afterSelectingRows() {
this.props.onUserSelection(
this.preSelectedRows.map(row => row.uniqueKey)
);
this.preSelectedRows = [];
this.props.onUserSelection(this.selectedRowsKeys);
this.selectedRowsKeys = [];
}

@computed
get selectedRows() {
if (this.props.filters.length === 0) {
return [];
} else {
return _.reduce(
this.tableData,
(
acc: GeneTableUserSelectionWithIndex[],
row,
index: number
) => {
if (_.includes(this.props.filters, row.uniqueKey)) {
acc.push({
rowIndex: index,
uniqueKey: row.uniqueKey
});
}
return acc;
},
[]
);
}
@autobind
isSelectedRow(data: GeneTableRow) {
return this.isChecked(data.uniqueKey);
}

@computed get filterKeyToIndexSet() {
return _.reduce(this.props.filters, (acc, next, index) => {
next.forEach(key => {
acc[key] = index;
});
return acc;
}, {} as { [id: string]: number });
}

@autobind
isSelectedRow(data: GeneTableRow) {
return !_.isUndefined(
_.find(_.union(this.selectedRows, this.preSelectedRows), function (row) {
return row.uniqueKey === data.uniqueKey;
})
);
selectedRowClassName(data: GeneTableRow) {
const index = this.filterKeyToIndexSet[data.uniqueKey];
if (index === undefined) {
return this.props.filters.length % 2 === 0 ? styles.highlightedEvenRow : styles.highlightedOddRow;
}
return index % 2 === 0 ? styles.highlightedEvenRow : styles.highlightedOddRow;
Copy link
Contributor

@adamabeshouse adamabeshouse Jan 15, 2020 •
edited
Loading

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

}

@autobind
Expand All @@ -498,14 +492,16 @@ export class GeneTable extends React.Component<GeneTableProps, {}> {
<GeneTableComponent
width={this.props.width}
height={this.props.height}
data={this.tableData}
data={this.selectableTableData}
columns={this.tableColumns}
showSelectSamples={true && this.preSelectedRows.length > 0}
showSelectSamples={true && this.selectedRowsKeys.length > 0}
isSelectedRow={this.isSelectedRow}
afterSelectingRows={this.afterSelectingRows}
sortBy={this.sortBy}
sortDirection={this.sortDirection}
afterSorting={this.afterSorting}
fixedTopRowsData={this.preSelectedRows}
highlightedRowClassName={this.selectedRowClassName}
/>
)}
{this.props.genePanelCache ? (
Expand Down
6 changes: 5 additions & 1 deletion src/pages/studyView/table/tables.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,14 @@ $study-view-table-icon-size: 10px;
background-color: #ffffff;
}

.highlightedRow {
.highlightedEvenRow, .highlightedRow {
background-color: #d3d3d3;
}

.highlightedOddRow {
background-color: #e0e0e0;
}

.cancerGeneIcon {
margin-right: 5px;
}
Expand Down