-
-
Notifications
You must be signed in to change notification settings - Fork 72
Issue 324 - Fix header misalignment regression with "always visible" scrollbar #325
Changes from all commits
432cbcf
b5d1ada
c2bf63d
2709cea
d3dde16
30816f9
93fe36d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,8 +24,11 @@ | |
| "private::wait_dash8083": "wait-on http://localhost:8083", | ||
| "private::wait_js": "wait-on http://localhost:8080", | ||
| "private::opentests": "cypress open", | ||
| "private::runtests-v0": "cypress run --browser chrome --spec 'tests/cypress/tests/server/**/*'", | ||
| "private::runtests-v1": "cypress run --browser chrome", | ||
| "private::runtests:unit": "cypress run --browser chrome --spec 'tests/cypress/tests/unit/**/*'", | ||
| "private::runtests:standalone": "cypress run --browser chrome --spec 'tests/cypress/tests/standalone/**/*'", | ||
| "private::runtests:server": "cypress run --browser chrome --spec 'tests/cypress/tests/server/**/*'", | ||
| "private::runtests-v0": "run-s private::runtests:server", | ||
| "private::runtests-v1": "run-s private::runtests:unit private::runtests:standalone private::runtests:server", | ||
| "build.watch": "webpack-dev-server --content-base dash_table --mode development", | ||
| "build": "run-s private::build:js private::build:py", | ||
| "lint": "tslint '{src,demo,tests}/**/*.{js,ts,tsx}' --exclude '**/@Types/*.*'", | ||
|
|
@@ -39,42 +42,42 @@ | |
| "license": "MIT", | ||
| "dependencies": {}, | ||
| "devDependencies": { | ||
| "@babel/cli": "^7.2.0", | ||
| "@babel/core": "^7.2.0", | ||
| "@babel/polyfill": "^7.0.0", | ||
| "@babel/preset-env": "^7.2.0", | ||
| "@babel/cli": "^7.2.3", | ||
| "@babel/core": "^7.2.2", | ||
| "@babel/polyfill": "^7.2.5", | ||
| "@babel/preset-env": "^7.2.3", | ||
| "@babel/preset-react": "^7.0.0", | ||
| "@cypress/webpack-preprocessor": "^4.0.2", | ||
| "@percy-io/percy-storybook": "^2.1.0", | ||
| "@storybook/cli": "^4.0.12", | ||
| "@storybook/react": "^4.0.12", | ||
| "@types/ramda": "^0.25.42", | ||
| "@types/react": "^16.7.13", | ||
| "@storybook/cli": "^4.1.4", | ||
| "@storybook/react": "^4.1.4", | ||
| "@types/ramda": "^0.25.46", | ||
| "@types/react": "^16.7.18", | ||
| "@types/react-select": "^1.3.4", | ||
| "babel-loader": "^8.0.4", | ||
| "core-js": "^2.6.0", | ||
| "css-loader": "^2.0.0", | ||
| "cypress": "^3.1.3", | ||
| "file-loader": "^2.0.0", | ||
| "babel-loader": "^8.0.5", | ||
| "core-js": "^2.6.1", | ||
| "css-loader": "^2.1.0", | ||
| "cypress": "^3.1.4", | ||
| "file-loader": "^3.0.1", | ||
| "http-server": "^0.11.1", | ||
| "less": "^3.9.0", | ||
| "less-loader": "^4.1.0", | ||
| "npm": "^6.4.1", | ||
| "npm": "^6.5.0", | ||
| "npm-run-all": "^4.1.5", | ||
| "ramda": "^0.26.1", | ||
| "react": "16.6.3", | ||
| "react": "16.7.0", | ||
| "react-docgen": "^2.21.0", | ||
| "react-dom": "16.6.3", | ||
| "react-dom": "16.7.0", | ||
| "react-select": "^1.3.0", | ||
| "sheetclip": "^0.3.0", | ||
| "style-loader": "^0.23.1", | ||
| "ts-loader": "^5.3.1", | ||
| "tslint": "^5.11.0", | ||
| "ts-loader": "^5.3.3", | ||
| "tslint": "^5.12.0", | ||
| "typescript": "^3.2.2", | ||
| "wait-on": "^3.2.0", | ||
| "webpack": "^4.27.1", | ||
| "webpack-cli": "^3.1.2", | ||
| "webpack-dev-server": "^3.1.10", | ||
| "webpack": "^4.28.3", | ||
| "webpack-cli": "^3.2.1", | ||
| "webpack-dev-server": "^3.1.14", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is noise from renovate/all -- we don't commit the build result when updating deps |
||
| "webpack-preprocessor": "^0.1.12" | ||
| }, | ||
| "files": [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,10 +16,12 @@ export default ( | |
| viewport: IDerivedData, | ||
| rowPadding: IViewportPadding, | ||
| scrollbarWidth: number | ||
| ): { fragment?: CSSProperties, cell?: CSSProperties}[][] => { | ||
| ): { fragment?: CSSProperties, cell?: CSSProperties }[][] => { | ||
| const base = [{}, { fragment: { marginRight: scrollbarWidth } }]; | ||
|
|
||
| if (!virtualization || !uiCell || !uiViewport) { | ||
| return [ | ||
| [{}, {}], | ||
| base, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is no virtualization or if the virtualization is not ready -- return the scrolling offset only |
||
| [{}, {}] | ||
| ]; | ||
| } | ||
|
|
@@ -28,12 +30,11 @@ export default ( | |
| const virtualizedHeight = (Math.floor(uiViewport.scrollTop / uiCell.height) - rowPadding.before) * uiCell.height; | ||
| const headersHeight = R.sum(R.map(h => h.height, uiHeaders || [])); | ||
|
|
||
| const marginRight = scrollbarWidth; | ||
| const marginTop = virtualization && uiViewport && uiCell ? Math.max(virtualizedHeight - headersHeight, 0) : 0; | ||
| const height = Math.max(fullHeight - marginTop, 0); | ||
|
|
||
| return [ | ||
| [{}, { fragment: { marginRight } }], | ||
| base, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise, return offset + virtualization-specific styling |
||
| [{ cell: { marginTop } }, { fragment: { height, marginTop } }] | ||
| ]; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import derivedFragmentStyles from 'dash-table/derived/table/fragmentStyles'; | ||
|
|
||
| function validate(style, width) { | ||
| expect(style.length).to.equal(2); | ||
| expect(style[0]).to.not.equal(undefined); | ||
| expect(style[1]).to.not.equal(undefined); | ||
| expect(style[0].length).to.equal(2); | ||
| expect(style[1].length).to.equal(2); | ||
| expect(style[0][1]).to.not.equal(undefined); | ||
| expect(style[0][1].fragment).to.not.equal(undefined); | ||
| expect(style[0][1].fragment.marginRight).to.equal(width); | ||
| } | ||
|
|
||
| describe('fragment styles', () => { | ||
| const cell = { height: 30 }; | ||
| const headers = []; | ||
| const padding = { before: 0, after: 0 }; | ||
| const scrollbarWidth = 20; | ||
| const uiViewport = { scrollTop: 0, scrollLeft: 0, height: 0, width: 0 }; | ||
| const viewport = { data: [], indices: [] }; | ||
|
|
||
| describe('of "ready" virtualized table', () => { | ||
| it('includes scrollbar width styling', () => { | ||
| const style: any = derivedFragmentStyles( | ||
| true, | ||
| cell, | ||
| headers, | ||
| uiViewport, | ||
| viewport, | ||
| padding, | ||
| scrollbarWidth | ||
| ); | ||
|
|
||
| validate(style, scrollbarWidth); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basic unit tests to make sure (not-)virtualized table (not-)ready always returns the correct styling for the header alignment / offset. |
||
| }); | ||
| }); | ||
|
|
||
| describe('of "inprogress" virtualized table', () => { | ||
| it('includes scrollbar width styling', () => { | ||
| const style: any = derivedFragmentStyles( | ||
| true, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| viewport, | ||
| padding, | ||
| scrollbarWidth | ||
| ); | ||
|
|
||
| validate(style, scrollbarWidth); | ||
| }); | ||
| }); | ||
|
|
||
| describe('of "ready" non-virtualized table', () => { | ||
| it('includes scrollbar width styling', () => { | ||
| const style: any = derivedFragmentStyles( | ||
| false, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| viewport, | ||
| padding, | ||
| scrollbarWidth | ||
| ); | ||
|
|
||
| validate(style, scrollbarWidth); | ||
| }); | ||
| }); | ||
|
|
||
| describe('of "inprogress" non-virtualized table', () => { | ||
| it('includes scrollbar width styling', () => { | ||
| const style: any = derivedFragmentStyles( | ||
| false, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| viewport, | ||
| padding, | ||
| scrollbarWidth | ||
| ); | ||
|
|
||
| validate(style, scrollbarWidth); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some annoying tidbits related to Cypress -- it seems the additional tests tipped it over into Heap Overflow land.