Issue 680 - row and Bootstrap interactions
#844
Conversation
…5.0.0alpha2 behavior
| @@ -1010,7 +1010,7 @@ export default class ControlledTable extends PureComponent< | |||
| <div | |||
| key={`r${rowIndex}`} | |||
| ref={`r${rowIndex}`} | |||
| className={`row row-${rowIndex}`} | |||
| className={`table-row table-row-${rowIndex}`} | |||
There was a problem hiding this comment.
Fixing the class collision with Bootstrap
| display: flex; | ||
| flex: 0 0 auto; | ||
| flex-direction: row; | ||
| } | ||
|
|
||
| .row-1 { | ||
| .table-row-1 { |
There was a problem hiding this comment.
Same class collision with Bootstrap
| @@ -3,6 +3,7 @@ import React from 'react'; | |||
| import { storiesOf } from '@storybook/react'; | |||
| import random from 'core/math/random'; | |||
| import DataTable from 'dash-table/dash/DataTable'; | |||
| import './common'; | |||
There was a problem hiding this comment.
Could technically add this to only one test and it would be applied to all but it seemed better to duplicate the import just so it's clear that this is "expected".
| @@ -0,0 +1,2 @@ | |||
| import 'bootstrap'; | |||
| import 'bootstrap/dist/css/bootstrap.css'; | |||
There was a problem hiding this comment.
Import all the bootstrap things. This is applied to all tests (whether they all import this file or not). As currently set up, this means that we are no longer testing a "vanilla" version of the table in these tests and we would be required to duplicate this test run to do so (e.g. an additional ~1600 snapshots per build)
There are some differences between vanilla and the bootstrap styled table, as far as I can tell the main difference seems to be a slightly different font size. Nothing that strikes as compromising the vanilla test cases.
| @@ -376,9 +376,7 @@ def test(request, dash_thread_server, tmpdir): | |||
| remote_url=request.config.getoption("remote_url"), | |||
| headless=request.config.getoption("headless"), | |||
| options=request.config.hook.pytest_setup_options(), | |||
| download_path=tmpdir.mkdir("download").strpath, | |||
| percy_assets_root=request.config.getoption("percy_assets"), | |||
| download_path=tmpdir.mkdir("dt-download").strpath, | |||
There was a problem hiding this comment.
Table tests sometimes use dash_thread_server and dash_duo -- dash_duo specifically also creates a temporary folder, both fixtures clash and the tests end up throwing an error because the directory already exists. Pytest uses a py27 compatible object that does not expose exists_ok and hence the error can't otherwise be prevented.
| **ops | ||
| ) | ||
|
|
||
| app.layout = html.Div([DataTable(**props)]) |
There was a problem hiding this comment.
Initially wanted this test to show a unmodified table + a another table in an iframe with bootstrap styles applied to offer some comparison point during review (e.g. only the bootstrap version changed vs. both) but the iframe content is never visible.
Fixes #680
Fixes #796
table-prefix to therowandrow-{number}classesline-heightchangesAlso adds visual tests to the
tests/seleniumtest run and adds the newpercy/dash-table-e2esnapshots bucket