-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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.
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
fix: FilterableTable result div width #16912
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16912 +/- ##
=======================================
Coverage 76.92% 76.92%
=======================================
Files 1030 1030
Lines 55022 55022
Branches 7465 7465
=======================================
Hits 42328 42328
Misses 12440 12440
Partials 254 254
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
@@ -435,7 +435,7 @@ export default class FilterableTable extends PureComponent< | |||
}} | |||
className={`grid-cell ${this.rowClassName({ index: rowIndex })}`} | |||
> | |||
<div>{content}</div> | |||
<div style={{ width: 'inherit' }}>{content}</div> |
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.
It would be better to move this to a styled component. You have it available in this file already. In general, we are trying to avoid inline styles even when they are minimal like this case.
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.
Heya, good catch! Since this is just a single-use style change I used css=
for this case, as that's what the Frontend Guidelines suggest for cases like this. Changed in this commit.
@geido Ephemeral environment spinning up at http://54.244.56.79:8080. Credentials are |
15fe929
to
ee45a44
Compare
/testenv up |
@eschutho Ephemeral environment spinning up at http://34.209.73.62:8080. Credentials are |
ee45a44
to
4fe63b1
Compare
/testenv up |
@eschutho Ephemeral environment spinning up at http://34.220.82.219:8080. Credentials are |
Ephemeral environment shutdown and build artifacts deleted. |
* FilterableTable result div width is now inherit * Changed style to css
🏷 v1.4 |
* FilterableTable result div width is now inherit * Changed style to css (cherry picked from commit 90cfa7f)
* FilterableTable result div width is now inherit * Changed style to css
SUMMARY
When a table exceeded 50 columns, some of the results would appear to be cut off. The results now show properly by setting the width of the div wrapping the result to
inherit
.max-content
butinherit
seemed safer.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE
AFTER
TESTING INSTRUCTIONS
Find/create a table that exceeds 50 columns and contains timestamps. Observe that the results are no longer cut off and display correctly.
superset load-examples -b
), you can repro this in SQL Lab's SQL editor with this query:SELECT * FROM wide_table
ADDITIONAL INFORMATION