[Table] Turn off text truncation by default and raise fixed truncation limit #1656

tgreenwatts · 2017-10-03T21:04:23Z

Benefits: No more detecting truncation, which is expensive. Performance benefits
Drawbacks: A mix of little … and big …s, but I think this will be uncommon and the big … can change meaning from “contents don’t fit cell” to “contents are really big”, also an icon change will help
Drawbacks: Wrapped text no longer gets … of any sort (unless real big). I’m fine with this, it’s pretty common. Apps that wrap should also resizeByTallest
-How we do it:
— Change the default from detectTruncation to fixedTruncation
— Choose sane default
— Write good release notes

I may have missed spots

gscshoyru

This is a breaking change; we may want to modify the defaults, but we shouldn't do that until we hit a major release, so we can call it out without breaking semver.

tgreenwatts · 2017-10-04T19:05:52Z

I believe this is a UX change, but no different than a UX change like adding reorder handles to the columns without an interaction bar - it's a change, but one we believe is better for everyone. This should not disrupt any users. We will call it out in release notes clearly so if anyone wants to opt in to detectTruncation they are welcome to.

Before:

After:

This column is too long for it's current width, but not so long that it can't reasonably be displayed, so we do not truncate.

adidahiya

Ok, I thought about this some more and it's a reasonable UX change to make if we have high conviction that the old UX was fairly broken (I think we do). As @tgreenwatts said, we'll call it out in release notes clearly.

tgreenwatts · 2017-10-04T21:56:01Z

I played with this in the dev preview and think the default was a little too low, so up-ed it to 2000.

blueprint-bot · 2017-10-04T22:32:21Z

Actually fix lint

Preview: documentation | table
Coverage: core | datetime

Removing concens

cmslewis · 2017-10-05T18:28:05Z

packages/table/src/cell/formats/jsonFormat.tsx

@@ -31,10 +31,11 @@ export interface IJSONFormatProps extends ITruncatedFormatProps {

 export class JSONFormat extends React.Component<IJSONFormatProps, {}> {
    public static defaultProps: IJSONFormatProps = {
-        detectTruncation: true,
+        detectTruncation: false,


Can delete this line and fall back to the default props defined in TruncatedFormat.

Wouldn't this be true for showPopover as well? it's default here is showPopover: TruncatedPopoverMode.WHEN_TRUNCATED, which is the default

cmslewis · 2017-10-05T18:28:12Z

packages/table/src/cell/formats/jsonFormat.tsx

        omitQuotesOnStrings: true,
        showPopover: TruncatedPopoverMode.WHEN_TRUNCATED,
        stringify: (obj: any) => JSON.stringify(obj, null, 2),
+        truncateLength: 2000,


cmslewis · 2017-10-05T18:29:30Z

packages/table/src/cell/formats/truncatedFormat.tsx

@@ -66,8 +66,8 @@ export interface ITruncatedFormatProps extends IProps {
    /**
     * Number of characters that are displayed before being truncated and appended with the
     * `truncationSuffix` prop. A value of 0 will disable truncation. This prop is ignored if
-     * `detectTruncation` is `true`.
-     * @default 80
+     * `detectTruncation` is `false`.


Hmm, should this have changed? See https://github.com/palantir/blueprint/pull/1656/files#diff-820e3fef53ee313553edff37ddbf4737R116. By that logic, we'll never refer to the truncateLength if detectTruncation is true.

cmslewis · 2017-10-05T18:29:52Z

packages/table/test/formatsTests.tsx

-                our inevitable return to that prior state from which the vast
-                majority have never stirred?
-            `;
+            const str = createStringOfLength(TruncatedFormat.defaultProps.truncateLength + 1);


Beau-tee-ful!

cmslewis · 2017-10-05T18:32:45Z

packages/table/test/formatsTests.tsx

@@ -71,11 +57,11 @@ describe("Formats", () => {
                majority have never stirred?
            `;

-            const style = { height: "200px", position: "relative" };
+            const style = { height: "300px", width: "300px", position: "relative" };


I'd love a comment here.

// fix the container's width and height to ensure this test passes // regardless of the page's dimensions.

cmslewis · 2017-10-05T18:43:35Z

@tgreenwatts - Wait wait, did you see my requested changes? Would like to see those implemented before merging. Reverting.

…runcation limit (#1656)" This reverts commit ae31cc3.

* Follow up to #1656 * Remove redundant defaults

Turn off text truncation by default and raise fixed truncation limit

a3b3483

tgreenwatts requested review from cmslewis, themadcreator and gscshoyru October 3, 2017 21:04

cmslewis changed the title ~~Turn off text truncation by default and raise fixed truncation limit~~ [Table] Turn off text truncation by default and raise fixed truncation limit Oct 3, 2017

cmslewis added Package: table Type: enhancement labels Oct 3, 2017

gscshoyru previously requested changes Oct 3, 2017

View reviewed changes

adidahiya approved these changes Oct 4, 2017

View reviewed changes

Fix tests and up default

2499c69

Taryn Green Watts added 2 commits October 4, 2017 15:05

Remove only and add newline

877e41c

Actually fix lint

c6adec6

cmslewis suggested changes Oct 5, 2017

View reviewed changes

tgreenwatts merged commit ae31cc3 into master Oct 5, 2017

cmslewis added a commit that referenced this pull request Oct 5, 2017

Revert "[Table] Turn off text truncation by default and raise fixed t…

115a0fb

…runcation limit (#1656)" This reverts commit ae31cc3.

tgreenwatts pushed a commit that referenced this pull request Oct 5, 2017

Follow up to #1656

89dd021

tgreenwatts mentioned this pull request Oct 5, 2017

[Table/TruncatedFormat] Follow up to #1656 #1670

Merged

tgreenwatts deleted the tgreen/fixed-truncation-default branch October 5, 2017 20:21

cmslewis pushed a commit that referenced this pull request Oct 5, 2017

[Table/TruncatedFormat] Follow up to #1656 (#1670)

0a9edf7

* Follow up to #1656 * Remove redundant defaults

cmslewis mentioned this pull request Oct 6, 2017

Prepare Release 1.31.0 #1687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Table] Turn off text truncation by default and raise fixed truncation limit #1656

[Table] Turn off text truncation by default and raise fixed truncation limit #1656

tgreenwatts commented Oct 3, 2017

gscshoyru left a comment

tgreenwatts commented Oct 4, 2017

adidahiya left a comment

tgreenwatts commented Oct 4, 2017

blueprint-bot commented Oct 4, 2017

cmslewis Oct 5, 2017

tgreenwatts Oct 5, 2017

cmslewis Oct 5, 2017

cmslewis Oct 5, 2017

cmslewis Oct 5, 2017

cmslewis Oct 5, 2017

cmslewis commented Oct 5, 2017

[Table] Turn off text truncation by default and raise fixed truncation limit #1656

[Table] Turn off text truncation by default and raise fixed truncation limit #1656

Conversation

tgreenwatts commented Oct 3, 2017

gscshoyru left a comment

Choose a reason for hiding this comment

tgreenwatts commented Oct 4, 2017

adidahiya left a comment

Choose a reason for hiding this comment

tgreenwatts commented Oct 4, 2017

blueprint-bot commented Oct 4, 2017

Actually fix lint

cmslewis Oct 5, 2017

Choose a reason for hiding this comment

tgreenwatts Oct 5, 2017

Choose a reason for hiding this comment

cmslewis Oct 5, 2017

Choose a reason for hiding this comment

cmslewis Oct 5, 2017

Choose a reason for hiding this comment

cmslewis Oct 5, 2017

Choose a reason for hiding this comment

cmslewis Oct 5, 2017

Choose a reason for hiding this comment

cmslewis commented Oct 5, 2017