CSS cleanups, fixes, and reimplementations #1780

nstepien · 2019-10-09T13:00:41Z

Some things to note:

cells are now centered vertically by setting the line-height === row height.
- This fixes header cells text not being vertically centered
- We were previously using position: absolute; top: 50%; transform: translateY(-50%) which may hurt text rendering or layout times.
The generic editors should now be perfectly aligned with the formatters
Selected rows now have a hover style
cell actions icons are missing, should we add a dependency on some icons?

qili26 · 2019-10-09T19:31:05Z

packages/react-data-grid/src/Cell/CellContent.tsx

+          isScrolling={isScrolling}
+          isRowSelected={isRowSelected}
+          onRowSelectionChange={onRowSelectionChange}
+        />
        {cellControls}
      </div>
      {tooltip && <span className="cell-tooltip-text">{tooltip}</span>}


https://github.com/adazzle/react-data-grid/pull/1773/files/cba04c56c295e32fa3f036cb6cd9dbd5f8d5ec60#r333000852

Thoughts about removing this here? Since the style is touched, if we want to remove, I bet it's better in this PR. Or we keep it for revisiting.

Let's do it in a separate PR.

amanmahajan7 · 2019-10-09T21:36:46Z

packages/react-data-grid/style/rdg-header.less

@@ -22,6 +22,13 @@
  width: 10px;
 }

+@supports (position: sticky) or (position: -webkit-sticky) {


Do we need this? Currently we render all the header cells and adjust the scrolleft on scroll

We also iterate through all the columns to set/unset the scroll position of each header row cells.

I figure we should optimize this for browsers that support sticky position, no?

amanmahajan7

A few cleanup comments. Cell actions example does not work, I think we need to replace the icons. May be we can import material icon for examples?

amanmahajan7 · 2019-10-11T19:41:21Z

packages/react-data-grid/src/common/editors/EditorContainer.tsx

@@ -39,10 +39,6 @@ export default class EditorContainer<R> extends React.Component<Props<R>, State>
    const inputNode = this.getInputNode();
    if (inputNode instanceof HTMLElement) {
      inputNode.focus();
-      if (!this.getEditor().disableContainerStyles) {


amanmahajan7 · 2019-10-11T19:44:42Z

packages/react-data-grid/src/HeaderCell.tsx


-    const className = classNames('react-grid-HeaderCell', {
+    const className = classNames('react-grid-Cell', {


Should we rename it to rdg-cell?

Sure, there's still a lot of opportunities to clean up the existing stylesheets, and rename classes, but I've mostly focused on replacing bootstrap in this PR.

amanmahajan7 · 2019-10-11T19:46:32Z

packages/react-data-grid/src/HeaderCell.tsx


-    const className = classNames('react-grid-HeaderCell', {
+    const className = classNames('react-grid-Cell', {
      'rdg-header-cell-resizable': column.resizable,
      'react-grid-HeaderCell--frozen': isFrozen(column)


Same with react-grid-HeaderCell

amanmahajan7 · 2019-10-11T19:48:09Z

packages/react-data-grid/src/HeaderRow.tsx

-        }}
-        className="react-grid-HeaderRow"
+        className="rdg-header-row"
+        style={{ height: this.props.height }}


I think the height (500) was needed to show the Dropdown filters

Yeah, imo we should just reuse the editor container for such custom filters, otherwise it makes it hard to work with overflow: hidden.

amanmahajan7 · 2019-10-11T19:49:53Z

packages/react-data-grid/style/rdg-cell.less

-  background-color: #66afe930;
+  white-space: nowrap;
+  overflow: hidden;
+  text-overflow: ellipsis;
 }

 .react-grid-Cell--frozen {
  /* Should have a higher value than 1 to show in front of cell masks */
  z-index: 2;
 }
 .rdg-last--frozen {


we can probably rename it to rdg-cell-last

amanmahajan7 · 2019-10-11T19:50:25Z

packages/react-data-grid/style/rdg-cell.less

-  overflow: hidden;
-  height: inherit;
-}
-
 .react-grid-Cell.readonly {


Do we need all the styles?

react-grid-Cell.readonly react-grid-cell .form-control-feedback

Probably not, I haven't fully cleaned up this file.

amanmahajan7 · 2019-10-11T19:50:39Z

packages/react-data-grid/style/rdg-checkbox.less

-}
-
-.react-grid-checkbox:checked + .react-grid-checkbox-label:before {
+.rdg-checkbox-label {


amanmahajan7 · 2019-10-11T19:53:32Z

packages/react-data-grid/style/rdg-header.less

-.react-grid-HeaderCell--frozen:last-of-type {
-    box-shadow: 2px 0 5px -2px rgba(136, 136, 136, .3);
+@supports (position: sticky) or (position: -webkit-sticky) {
+  .react-grid-HeaderCell--frozen {


we can raname these classes as well

qili26 · 2019-10-14T16:05:21Z

packages/react-data-grid/src/common/cells/headerCells/SortableHeaderCell.tsx

      {content}
+      <span className="rdg-header-sort">{SORT_TEXT[sortDirection]}</span>


Since we changed to MUI in this PR, should we change this as well to be an icon?

Let's leave it like this for now.

qili26 · 2019-10-15T02:39:40Z

packages/react-data-grid-addons/src/draggable/RowDragLayer.js

@@ -69,9 +69,9 @@ class CustomDragLayer extends Component {
          if (c.formatter) {
            const Formatter = c.formatter;
            const dependentValues = typeof c.getRowMetaData === 'function' ? c.getRowMetaData(item, c) : {};
-            cells.push(<td key={`dragged-cell-${rowIdx}-${c.key}`} className="react-grid-Cell" style={{ padding: '5px' }}><Formatter dependentValues={dependentValues} value={item[c.key]} /></td>);
+            cells.push(<td key={`dragged-cell-${rowIdx}-${c.key}`} className="rdg-cell" style={{ padding: '5px' }}><Formatter dependentValues={dependentValues} value={item[c.key]} /></td>);


{ padding: 5 }? Any insights why this is not moved to css?

This PR doesn't aim to fix every styling issues in the repo, it's just trying to address issues that arose after removing the bootstrap styles. There are too many things to fix or cleanup otherwise.

qili26 · 2019-10-15T02:40:30Z

packages/react-data-grid/style/rdg-editor.less

+  -webkit-appearance: none;
+  appearance: none;
+
+  box-sizing: border-box;


do we need to specify this?

Yes, it's too big otherwise.

qili26 · 2019-10-15T02:45:25Z

packages/react-data-grid/src/Header.tsx

@@ -124,17 +122,12 @@ export default forwardRef(function Header<R>(props: HeaderProps<R>, ref: React.R
    props.cellMetaData.onCellClick({ rowIdx: -1, idx: -1 });
  }

-  const className = classNames('react-grid-Header', {
-    'react-grid-Header--resizing': resizing !== null


no more this?

Doesn't look like we need it no.

qili26 · 2019-10-15T02:52:44Z

packages/react-data-grid/style/rdg-header.less

    color: #555555;
 }

 .rdg-sortable-header-cell {
  cursor: pointer;
 }
+
+.rdg-header-sort {


maybe using flex and put 2 divs inside rather than just float: right?

It just works though?

My concern is that when the columns width is 100px while the header text width is like 200px, will the sort icon being pushed to next line or keep in the view. I believe using float: right will push it to next line. I am trying to run this PR in my local but having some packages installing issue.

Clean up a bit of the core css, and Header type

87c520c

nstepien requested a review from amanmahajan7 October 9, 2019 13:00

nstepien self-assigned this Oct 9, 2019

nstepien added 2 commits October 9, 2019 16:01

Row css cleanup

1dd80a0

Fix fragment id prop warning in dev mode

0f4217b

nstepien force-pushed the ns-css branch from 60795e0 to 0f4217b Compare October 9, 2019 15:11

context menu styling

4a1539a

nstepien changed the title ~~CSS cleanup~~ CSS cleanups, fixes, and reimplementations Oct 9, 2019

nstepien added 3 commits October 9, 2019 16:53

Fix missing background color on frozen cells

f1831ae

Use flex to align cells vertically, small optimizations

3274cdc

header cells = cells

68319b5

qili26 reviewed Oct 9, 2019

View reviewed changes

Use sticky position for frozen header cells as well

8f7b035

amanmahajan7 reviewed Oct 9, 2019

View reviewed changes

nstepien and others added 15 commits October 9, 2019 23:52

Fix tests

847aaa4

Editor styling

f1bfc57

Fix tests

0946abc

bit more padding

8df3097

small fixes

39e3df2

Merge remote-tracking branch 'origin/canary' into ns-css

e551156

Redo checkbox styling

0b62bf3

Merge remote-tracking branch 'origin/canary' into ns-css

d1d0632

Merge branch 'canary' into ns-css

a798e63

Merge branch 'canary' into ns-css

a71bb68

more draggable header cell fixes

2655198

fix NumericFilter style

69e0d31

Align cells vertically using line-height

9a2039b

pull header sort arrows to the right

1c726b2

Fix tests

6992bc7

nstepien marked this pull request as ready for review October 11, 2019 18:18

nstepien added 2 commits October 11, 2019 20:19

set line-height on the root

1733bf2

Couple cleanups

fae14e1

amanmahajan7 reviewed Oct 11, 2019

View reviewed changes

nstepien added 7 commits October 12, 2019 00:27

rdg-cell

b24647b

more renaming

c7be8ee

center filters again

fe6abab

Add MUI icons package

242f370

Replace glyphicons

ccb75df

harmonize frozen cell styles

644eb2a

Fix jest performance

1616b74

qili26 reviewed Oct 14, 2019

View reviewed changes

qili26 reviewed Oct 15, 2019

View reviewed changes

nstepien and others added 5 commits October 15, 2019 12:40

Merge branch 'canary' into ns-css

54d387d

Scroll the header as a whole, skip some refs

51bb092

Fix tests

5f6e2f6

Fix sortable header cell text overflow

ed99436

Fix tests

10b0028

qili26 approved these changes Oct 15, 2019

View reviewed changes

nstepien merged commit a007e17 into canary Oct 15, 2019

nstepien deleted the ns-css branch October 15, 2019 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSS cleanups, fixes, and reimplementations #1780

CSS cleanups, fixes, and reimplementations #1780

nstepien commented Oct 9, 2019 •

edited

Loading

qili26 Oct 9, 2019

nstepien Oct 9, 2019

amanmahajan7 Oct 9, 2019

nstepien Oct 9, 2019

amanmahajan7 left a comment

amanmahajan7 Oct 11, 2019

amanmahajan7 Oct 11, 2019

nstepien Oct 11, 2019

amanmahajan7 Oct 11, 2019

amanmahajan7 Oct 11, 2019

nstepien Oct 12, 2019

amanmahajan7 Oct 11, 2019

amanmahajan7 Oct 11, 2019

nstepien Oct 11, 2019

amanmahajan7 Oct 11, 2019

amanmahajan7 Oct 11, 2019

qili26 Oct 14, 2019

nstepien Oct 15, 2019

qili26 Oct 15, 2019 •

edited

Loading

nstepien Oct 15, 2019

qili26 Oct 15, 2019

nstepien Oct 15, 2019

qili26 Oct 15, 2019

nstepien Oct 15, 2019

qili26 Oct 15, 2019

nstepien Oct 15, 2019

qili26 Oct 15, 2019


		const className = classNames('react-grid-HeaderCell', {
		const className = classNames('react-grid-Cell', {

		{content}
		<span className="rdg-header-sort">{SORT_TEXT[sortDirection]}</span>

CSS cleanups, fixes, and reimplementations #1780

CSS cleanups, fixes, and reimplementations #1780

Conversation

nstepien commented Oct 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanmahajan7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qili26 Oct 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nstepien commented Oct 9, 2019 •

edited

Loading

qili26 Oct 15, 2019 •

edited

Loading