feat(table): add tooltips to buttons and shrink icon size to 16 px, upgrade Carbon to v10.12.0 #1238

scottdickerson · 2020-05-28T20:50:27Z

Closes #975
Closes #1174
Closes https://github.ibm.com/Watson-IoT/pal-tracking/issues/399
Closes #1080

Summary

Fixes the toolbar buttons to be 16 px instead of 20, to use the official Carbon icon only buttons and to support tooltips. Also adds an active state to the button.

Change List (commits, features, bugs, etc)

chore(carbon): upgrade carbon to latest version 10.12.0 to get TableToolbarSearch fix
chore(package.json): increase the heap size for storybook (change for new carbon)
test(*Snapshots): lots of snapshot updates due to the new carbon
test(StorySnapshotsTest): needed to exclude two new DatePickerInput propTypes errors (opened a bug on Carbon to fix)
test(Table.story): fix downloadCSVHandler button to actually work in the story
test(Table.test): new Button elements no longer listen to onKeyDown, but do work with the enter key
refactor(TableToolbar): change back to Carbon TableToolbarSearch widget, use default Search icon size. Remove local copy of TableToolbarSearch.
fix(TableToolbarSVGButton): use official Carbon iconOnly button, support active state
chore(TileGallerySection): remove the Expand/Collapse i18n (change for new carbon)
fix(Table): allow overflow so that the tooltips can show at the top of the table
fix(baseTableReducer): because the search propType was renamed to 'defaultValue' from 'value' in a previous track need to use that in the reducer otherwise we will get a propTypes error after search

Acceptance Test (how to verify the PR)

Verify in this story watson-iot-table--stateful-example-with-expansion-maxpages-and-column-resize that the tooltips, and active button states work correctly. Verify the keyboard support of all the toolbar buttons

davidicus · 2020-05-28T21:06:03Z

package.json

@@ -86,8 +86,8 @@
    "@carbon/layout": "10.7.1",
    "@carbon/motion": "10.6.0",
    "@carbon/themes": "10.10.3",
-    "carbon-components": "10.10.3",
-    "carbon-components-react": "7.10.3",
+    "carbon-components": "^10.12.0",


These should be pinned to specific version.

Suggested change

"carbon-components": "^10.12.0",

"carbon-components": "10.12.0",

package.json

netlify · 2020-05-28T21:09:49Z

Deploy preview for carbon-addons-iot-react ready!

Built with commit d4c65b8

https://deploy-preview-1238--carbon-addons-iot-react.netlify.app

netlify · 2020-05-28T21:15:44Z

Deploy preview for carbon-addons-iot-react ready!

Built with commit 05764e4

https://deploy-preview-1238--carbon-addons-iot-react.netlify.app

bjornalm · 2020-05-28T21:17:11Z

src/components/Table/TableToolbar/_table-toolbar.scss

@@ -26,6 +20,11 @@ div.#{$prefix}--toolbar-action.#{$prefix}--toolbar-search-container-expandable {
  padding-top: $spacing-01;
 }

+// small fix to allow tooltips to show outside the toolbar if the batch action isn't active


Very nice, I was just fighting the clip-path myself, trying to figure out how to work around it without causing a breaking change. So you are sure it was only needed for the batch actions and not for anything else? What about custom toolbar content that might rely on it?

I believe so @bjornalm I almost opened a bug on carbon to only set the clip-path initially if the batch actions was active to make sure they approved.

tay1orjones · 2020-05-28T21:37:58Z

src/utils/__tests__/__snapshots__/publicAPI.test.js.snap

-    "defaultProps": Object {
-      "accept": Array [],
-      "buttonKind": "primary",
-      "disableLabelChanges": false,
-      "disabled": false,
-      "labelText": "Add file",
-      "multiple": false,
-      "onChange": [Function],
-      "onClick": [Function],
-      "role": "button",
-      "tabIndex": 0,
-    },


We should look into Carbon changelogs to see why this has changed.

tay1orjones · 2020-05-28T21:42:20Z

package.json

@@ -35,7 +35,7 @@
    "lint:stylelint": "stylelint './src/**/*.scss' --syntax scss --ignorePath .gitignore",
    "prepare": "yarn build",
    "publish-npm": "yarn semantic-release",
-    "start": "yarn test:engines && yarn storybook",
+    "start": "yarn test:engines && NODE_OPTIONS='--max-old-space-size=16384' yarn storybook",


Could you try a killall node -9 and then try running this command?

I assume you saw a heap out of memory error and added this. These are typically a result of resource issues on the local machine vs a reproducible issue across other machines. We shouldn't set this flag unless necessary.

yeah, it's fine if you wanna take it out, but I couldn't even get my initial build to work without it, and as far as I could tell I didn't have anything else eating local memory

no luck with the killall node -9 even after reboot, I couldn't do the yarn start without upping the heapsize! not sure if something is strange with the new carbon.

Co-authored-by: David Conner <david@david-conner.com>

stuckless

A couple things I noticed on the following story
https://deploy-preview-1238--carbon-addons-iot-react.netlify.app/?path=/story/watson-iot-table--with-resize-hascolumnselection-and-initial-column-widths

Do we want the tooltips to appear immediately (as they do), to on a timer on hover (like most tooltips in html). Is there anything in the design for that. I can see lots of comments coming back from users with inconsistencies if we do it immediately here on these buttons, but, don't do it immediately on all buttons, etc, or just the annoyance of it being immediate. Once you know the buttons, having the tooltips is more annoying than helpful.
The tooltips clip off the screen. It's pretty much a guarantee that tooltips will will larger descriptions esp in different languages, and that these buttons are going to pushed to the edge of the screen (in both rtl and ltr environments)
The entire window body adds in horz scrollbar. This is ugly, and it's not like we can actually scroll the page to see the tooltip, since, as soon as you mouse away the toolttip is gone.

Almost like we need a universal tooltip manager to mange using tooltips in all cases.

…bon-addons-iot-react into fix-table-toolbarbuttons

scottdickerson · 2020-05-29T11:56:27Z

A couple things I noticed on the following story
https://deploy-preview-1238--carbon-addons-iot-react.netlify.app/?path=/story/watson-iot-table--with-resize-hascolumnselection-and-initial-column-widths

Do we want the tooltips to appear immediately (as they do), to on a timer on hover (like most tooltips in html). Is there anything in the design for that. I can see lots of comments coming back from users with inconsistencies if we do it immediately here on these buttons, but, don't do it immediately on all buttons, etc, or just the annoyance of it being immediate. Once you know the buttons, having the tooltips is more annoying than helpful.

The tooltips clip off the screen. It's pretty much a guarantee that tooltips will will larger descriptions esp in different languages, and that these buttons are going to pushed to the edge of the screen (in both rtl and ltr environments)

The entire window body adds in horz scrollbar. This is ugly, and it's not like we can actually scroll the page to see the tooltip, since, as soon as you mouse away the toolttip is gone.

Almost like we need a universal tooltip manager to mange using tooltips in all cases.

@CynZhang what do you think?

src/components/Table/Table.test.jsx

…rusive

scottdickerson · 2020-05-29T17:18:29Z

ok @stuckless resubmitted after talking with @CynZhang and we gonna go with the native browser tooltips for now until Carbon gives us more options to make the default tooltips less intrusive.

scottdickerson · 2020-05-29T18:56:29Z

@CynZhang in the TableCard the icon size no longer matches once it's next to the CardIcons, should we update the CardIcon size as well?

CynZhang · 2020-05-29T19:37:55Z

@scottdickerson yes, those icons in the toolbar should all be 16px. Can you also check if the icon colors are the same?

tay1orjones · 2020-06-01T19:57:22Z

yarn.lock

+use-resize-observer@^6.0.0:
+  version "6.1.0"
+  resolved "https://registry.yarnpkg.com/use-resize-observer/-/use-resize-observer-6.1.0.tgz#d4d267a940dbf9c326da6042f8a4bb8c89d29729"
+  integrity sha512-SiPcWHiIQ1CnHmb6PxbYtygqiZXR0U9dNkkbpX9VYnlstUwF8+QqpUTrzh13pjPwcjMVGR+QIC+nvF5ujfFNng==
+  dependencies:
+    resize-observer-polyfill "^1.5.1"
+


This is coming from Carbon. We use an older version. It would be very beneficial if we updated our version of this dep to also use v6.1.

I've created a separate issue #1250 but we can include here if you'd prefer @scottdickerson

tay1orjones

#1253 Will fix the heap out of memory issue - lets wait to merge this until that one is in.

#1253 (comment):
A bit more context on this heap out of memory issue: it's a storybook issue with terser and react-docgen. It should be totally resolved in v6, but upgrading to Node 12 is a workaround.

scottdickerson · 2020-06-01T22:52:40Z

@tay1orjones thanks for the fix, I put in the PR with a dependency on yours, and it should be ready to go now

…fix-table-toolbarbuttons

tay1orjones

Node 12 PR is in, which should resolve the heap error we saw.

I pulled down the branch and verified locally - yarn start works fine on Node 12.17.0 👍

tay1orjones · 2020-06-02T20:46:08Z

yarn.lock

+carbon-components-react@7.12.0:
+  version "7.12.0"
+  resolved "https://registry.yarnpkg.com/carbon-components-react/-/carbon-components-react-7.12.0.tgz#d41125e774fa40e5c5fbc1d536c1923111273f6b"
+  integrity sha512-zsVD9/Dh8Rgl9LOO7vltS8AXj14eCbmSddQrDMkHeq5JSpQWtIqUAI7A7sT2uAhmowi1hk8d0BAroyGzhLHzAg==
  dependencies:
-    "@carbon/icons-react" "^10.9.3"
+    "@carbon/icons-react" "^10.11.0"


Looks like we should've upgraded icons-react too. I've opened a new PR: #1263

tay1orjones · 2020-06-02T20:47:16Z

🎉 This PR is included in version 2.84.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

scottdickerson added 5 commits May 27, 2020 17:42

fix(table): set toolbar buttons to correct size

0d895ea

chore(carbon): upgrade carbon components

5289496

feat(tabletoolbar): show active state of buttons

32b77ce

fix(toolbartooltips): hide the tooltip if the button is active

cb28008

fix(tabletoolbar): allow overflow on top of table

d4c65b8

scottdickerson requested review from blechner, bryancboyd, cgirani, davidicus, filipecorrea, jessieyan, leetosc, sstone2423, stuckless, tay1orjones and tomklapiscak as code owners May 28, 2020 20:50

test(tilegallery): remove expand/collapse i18n no longer used

6a8334b

davidicus reviewed May 28, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

bjornalm reviewed May 28, 2020

View reviewed changes

tay1orjones reviewed May 28, 2020

View reviewed changes

scottdickerson and others added 2 commits May 28, 2020 16:56

Update package.json

c24475b

Co-authored-by: David Conner <david@david-conner.com>

Merge branch 'master' into fix-table-toolbarbuttons

1979338

stuckless suggested changes May 29, 2020

View reviewed changes

scottdickerson added 2 commits May 29, 2020 06:46

fix(package): undo heap size

c798956

Merge branch 'fix-table-toolbarbuttons' of https://github.com/IBM/car…

7f1d48b

…bon-addons-iot-react into fix-table-toolbarbuttons

davidicus reviewed May 29, 2020

View reviewed changes

src/components/Table/Table.test.jsx Show resolved Hide resolved

fix(tabletoolbar): switch to default tooltip as carbon one is too int…

e6070ac

…rusive

scottdickerson requested a review from stuckless May 29, 2020 17:18

stuckless approved these changes May 29, 2020

View reviewed changes

tay1orjones changed the title ~~feat(table): add tooltips to buttons and shrink icon size to 16 px~~ feat(table): add tooltips to buttons and shrink icon size to 16 px, upgrade Carbon to v10.12.0 Jun 1, 2020

tay1orjones reviewed Jun 1, 2020

View reviewed changes

tay1orjones mentioned this pull request Jun 1, 2020

feat(project): support for node 12 #1253

Merged

tay1orjones requested changes Jun 1, 2020

View reviewed changes

fix(cardbuttons): the sizes need to match table button

4d590cd

scottdickerson requested a review from tay1orjones June 1, 2020 22:51

tay1orjones mentioned this pull request Jun 2, 2020

Update to Carbon v10.11.x #1080

Closed

Merge branch 'master' of github.com:IBM/carbon-addons-iot-react into …

e8474a4

…fix-table-toolbarbuttons

tay1orjones approved these changes Jun 2, 2020

View reviewed changes

Merge branch 'master' into fix-table-toolbarbuttons

05764e4

scottdickerson mentioned this pull request Jun 2, 2020

fix(dashboardactions): show tooltips over dashboard actions and use button #1261

Merged

tay1orjones merged commit 2120518 into master Jun 2, 2020

tay1orjones deleted the fix-table-toolbarbuttons branch June 2, 2020 20:26

tay1orjones mentioned this pull request Jun 2, 2020

feat(deps): update @carbon/icons-react to v10.11.0 #1263

Merged

tay1orjones reviewed Jun 2, 2020

View reviewed changes

tay1orjones added the released label Jun 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(table): add tooltips to buttons and shrink icon size to 16 px, upgrade Carbon to v10.12.0 #1238

feat(table): add tooltips to buttons and shrink icon size to 16 px, upgrade Carbon to v10.12.0 #1238

scottdickerson commented May 28, 2020 •

edited by tay1orjones

Loading

davidicus May 28, 2020

netlify bot commented May 28, 2020

netlify bot commented May 28, 2020 •

edited

Loading

bjornalm May 28, 2020

scottdickerson May 28, 2020

tay1orjones May 28, 2020

tay1orjones May 28, 2020

scottdickerson May 28, 2020

scottdickerson May 29, 2020

stuckless left a comment

scottdickerson commented May 29, 2020

scottdickerson commented May 29, 2020

scottdickerson commented May 29, 2020

CynZhang commented May 29, 2020

tay1orjones Jun 1, 2020

tay1orjones left a comment •

edited

Loading

scottdickerson commented Jun 1, 2020

tay1orjones left a comment

tay1orjones Jun 2, 2020

tay1orjones commented Jun 2, 2020

	"carbon-components": "^10.12.0",
	"carbon-components": "10.12.0",

feat(table): add tooltips to buttons and shrink icon size to 16 px, upgrade Carbon to v10.12.0 #1238

feat(table): add tooltips to buttons and shrink icon size to 16 px, upgrade Carbon to v10.12.0 #1238

Conversation

scottdickerson commented May 28, 2020 • edited by tay1orjones Loading

Choose a reason for hiding this comment

netlify bot commented May 28, 2020

netlify bot commented May 28, 2020 • 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

stuckless left a comment

Choose a reason for hiding this comment

scottdickerson commented May 29, 2020

scottdickerson commented May 29, 2020

scottdickerson commented May 29, 2020

CynZhang commented May 29, 2020

Choose a reason for hiding this comment

tay1orjones left a comment • edited Loading

Choose a reason for hiding this comment

scottdickerson commented Jun 1, 2020

tay1orjones left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tay1orjones commented Jun 2, 2020

scottdickerson commented May 28, 2020 •

edited by tay1orjones

Loading

netlify bot commented May 28, 2020 •

edited

Loading

tay1orjones left a comment •

edited

Loading