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

Cell Tooltip - Focus Issues #1422

Merged
merged 8 commits into from
Dec 6, 2018
Merged

Cell Tooltip - Focus Issues #1422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c576707
change level tooltip gets rendered and level it gets triggered
Nov 21, 2018
3b22f60
remove unsed editors and fix imports.
Nov 21, 2018
b7bdad8
re order inclusion of frozen cells to avoid z-index css.
Nov 21, 2018
108bae6
address comments
Nov 22, 2018
9fc3f17
Revert "remove unsed editors and fix imports."
Dec 5, 2018
71a2931
fix issues
Dec 5, 2018
a9e19fb
fix issues
Dec 5, 2018
714ad2a
remove extra space
Dec 5, 2018
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
15 changes: 10 additions & 5 deletions packages/react-data-grid/src/Cell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class Cell extends React.PureComponent {
let extraClasses = joinClasses({
'row-selected': this.props.isRowSelected,
editing: this.isEditorEnabled(),
'cell-tooltip': this.props.tooltip ? true : false,
'has-tooltip': this.props.tooltip ? true : false,
'rdg-child-cell': this.props.expandableOptions && this.props.expandableOptions.subRowDetails && this.props.expandableOptions.treeDepth > 0,
'last-column': this.props.column.isLastColumn
});
Expand Down Expand Up @@ -317,13 +317,20 @@ class Cell extends React.PureComponent {
if (treeDepth > 0 && isExpandCell) {
cellDeleter = <ChildRowDeleteButton treeDepth={treeDepth} cellHeight={this.props.height} siblingIndex={this.props.expandableOptions.subRowDetails.siblingIndex} numberSiblings={this.props.expandableOptions.subRowDetails.numberSiblings} onDeleteSubRow={this.onDeleteSubRow} isDeleteSubRowEnabled={isDeleteSubRowEnabled} />;
}

const tooltip = this.props.tooltip && (<span className="cell-tooltip-text">{this.props.tooltip}</span>);
const classes = joinClasses('react-grid-Cell__value',
{ 'cell-tooltip': this.props.tooltip ? true : false }
);

return (
<div className="react-grid-Cell__value">
<div className={classes}>
{cellDeleter}
<div style={{ marginLeft, position: 'relative', top: '50%', transform: 'translateY(-50%)' }}>
<span>{CellContent}</span>
{this.props.cellControls}
</div>
{tooltip}
</div>
);
};
Expand All @@ -347,8 +354,7 @@ class Cell extends React.PureComponent {
isScrolling
});

let events = this.getEvents();
const tooltip = this.props.tooltip ? (<span className="cell-tooltip-text">{this.props.tooltip}</span>) : null;
const events = this.getEvents();

const cellExpander = this.canExpand() && (
<CellExpand expandableOptions={this.props.expandableOptions} onCellExpand={this.onCellExpand} />
Expand All @@ -365,7 +371,6 @@ class Cell extends React.PureComponent {
{cellActionButtons}
{cellExpander}
{cellContent}
{tooltip}
Copy link
Contributor

@amanmahajan7 amanmahajan7 Dec 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I am still in favor of using a portals for tooltips. To answer your questions from the previous PR

  1. React uses custom events and use delegation so adding mouse events on each cell will not affect performance. React attaches event handlers on the document to handle this
  2. We can make tooltips flexible by allowing users to specify tooltip component and just align it with the cell. I will create a POC if I have time

</div>
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-data-grid/src/Row.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Row extends React.Component {
const { colOverscanStartIdx, colOverscanEndIdx, columns } = this.props;
const frozenColumns = columns.filter(c => columnUtils.isFrozen(c));
const nonFrozenColumnsToRender = columns.slice(colOverscanStartIdx, colOverscanEndIdx + 1).filter(c => !columnUtils.isFrozen(c));
return frozenColumns.concat(nonFrozenColumnsToRender)
return nonFrozenColumnsToRender.concat(frozenColumns)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it OK to change the frozen vs non-frozen order?

.map(column => this.getCell(column));
};

Expand Down
7 changes: 0 additions & 7 deletions packages/react-data-grid/src/__tests__/Row.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,6 @@ describe('Row', () => {
const renderedRange = colOverscanEndIdx - colOverscanStartIdx + 1;
expect(cells.length).toBe(renderedRange);
});

it('first rendered cell index should be first frozen cell', () => {
const columns = lockColumns(LAST_LOCKED_CELL_IDX);
const {cells} = setup({...requiredProperties, columns});
const firstFrozenColumn = columns.filter(c => c.frozen === true)[0];
expect(cells.first().props().column).toBe(firstFrozenColumn);
});
});

describe('When not using frozen columns', () => {
Expand Down
28 changes: 14 additions & 14 deletions themes/react-data-grid-cell.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@
box-shadow: 2px 0 5px -2px rgba(136, 136, 136, .3) !important;
}

/* cell which have tooltips need to have a higher z-index on hover so that the tooltip appears above the other cells*/
.react-grid-Cell.has-tooltip:hover {
z-index: 5;
Copy link
Contributor

@amanmahajan7 amanmahajan7 Dec 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

These numbers match the constant values . We need to change it once rc is merged to master

}

.react-grid-Cell.react-grid-Cell--frozen.has-tooltip:hover {
z-index: 15;
}

.react-contextmenu--visible {
z-index: 1000;
}
Expand Down Expand Up @@ -90,16 +99,8 @@
padding: 0;
}

.cell-tooltip {
position: relative;
display: inline-block;
}

.cell-tooltip:hover {
z-index: 101;
}

.cell-tooltip .cell-tooltip-text {
white-space: normal;
visibility: hidden;
width: 150px;
background-color: black;
Expand All @@ -108,12 +109,11 @@
border-radius: 6px;
padding: 5px 0;
position: absolute;
z-index: 1;
bottom: -150%;
top: 50%;
bottom: initial;
left: 50%;
margin-left: -60px;
/* Fade in tooltip - takes 1 second to go from 0% to 100% opacity */
opacity: 1s;
margin-top: 15px;
margin-left: -75px
}

.cell-tooltip:hover .cell-tooltip-text {
Expand Down