System 360 master #354

wonilsuh · 2017-10-09T21:00:01Z

Overview

This request includes updates to the following areas and components with the System 360 motion specs:

Global motion variables

The following variables in the following files were changes to the System 360 specs:

// src/components/button/_mixins.scss
$bx--ease-in
$bx--ease-out
$bx--standard-easing
$transition--base

Added

// src/globals/scss/_vars.scss
$bx--ease-in-out

`@ibm/motion` package

@ibm/motion NPM package was imported as a dev dependency to ensure that the motion definitions are up to date and in sync with other codebases that use the System 360 motion.

added as a dev dependency
a proxy was created under src/globals/js/misc/motion-generator.js

Button

Updated bezier curve according to the System 360 specs
Updated duration according to the System 360 specs

// src/components/button/_mixins.scss
transition-timing-function

Dropdown

Implements the full System 360 motion definitions

Updated bezier curve
Dynamic duration that changes according to the size of the menu
Caret motion was updated to match the System 360 motion
Implements cascading motion to menu items
Updated the demo HTML for a better demonstration of the dynamic durations

Overflow Menu / Floating Menu

Implements the full System 360 motion definition.

Updated bezier curve
Dynamic duration that changes according to the size of the menu
Implements cascading motion to menu items
- Updated the demo HTML for a better demonstration of the dynamic durations

Toggle

Implements the System 360 motion definition.

Updated bazier curve and duration

Button motion

…ponents into wonil-master

Easing curves

…into wonil-master

fix(style): added button active states

tay-aitken · 2017-10-10T13:19:54Z

Hi @wonilsuh can you please add a staging link so we can do visual reviews of this too? All component PRs need too design reviews and two code reviews.

asudoh

@wonilsuh Thank you for trying to contribute! Quickly reviewed the JavaScript code and put comments.

Besides the comment I made to the specific lines - My bigger question is if this added motion is something that should be applied to all Carbon users (e.g. Bluemix), or specific apps like the PR title seems to imply. I believe core Carbon team would have better ideas on this topic and interested in what they think.

asudoh · 2017-10-11T10:03:06Z

src/components/dropdown/dropdown.js

+
+          // iterate through children and accumulate height
+          for(let childrenIterator = 0; childrenIterator < listEl.children.length; childrenIterator++){
+            itemHeightTotal += listEl.children[childrenIterator].offsetHeight;


This line causes forced synchronous layout. It’d be great if we don’t have to get the dimension via JavaScript. Otherwise the code should run asynchronously e.g. in rAF callback.

Hi Asudo, I understand your concern regarding forced synchronous layout. The main reason for using this approach is to use duration that changes depending on the size of the element, which ensures smoother and consistent motion across experiences and products. I'll be committing slightly revised code shortly, but please let me know your thoughts on this.

asudoh · 2017-10-11T10:04:48Z

src/components/floating-menu/floating-menu.js

+        )
+      ;
+      this.element.style.transitionDuration = `${duration}ms`;
+      this.element.style.height = `${0}px`;


This line does not seem to require a template string.

asudoh · 2017-10-11T10:05:36Z

src/components/overflow-menu/overflow-menu.js

-    this.optionMenu.changeState(state, Object.assign(detail, { delegatorNode: this.element }), callback);
+    this.optionMenu.changeState(state, Object.assign(detail, { delegatorNode: this.element }), () => {
+
+      // if(state === 'shown'){


Please remove those commented-out lines.

asudoh · 2017-10-11T10:05:49Z

src/components/overflow-menu/overflow-menu.js

@@ -83,6 +105,8 @@ class OverflowMenu extends mixin(createComponent, initComponentBySearch, evented
    const state = shouldBeOpen ? 'shown' : 'hidden';

    if (isOfSelf) {
+      console.log(`OverflowMenu._handleDocumentClick...shouldBeOpen===${shouldBeOpen}, isOfSelf===${isOfSelf}, classList===${element.classList}`, this.element);


Please remove this debugging code.

asudoh · 2017-10-11T10:06:11Z

src/components/floating-menu/floating-menu.js

+      this.element.style.visibility = 'hidden';
+      this.element.style.height = 'auto';
+      let 
+        targetHeight = this.element.offsetHeight,


This line causes forced synchronous layout. It’d be great if we don’t have to get the dimension via JavaScript. Otherwise the code should run asynchronously e.g. in rAF callback.

asudoh · 2017-10-12T00:02:29Z

#354 (review)

CC @tw15egan @marijohannessen

asudoh · 2017-10-12T04:07:26Z

package-lock.json

@@ -0,0 +1,11859 @@
+{


I believe we are not putting package-lock.json in the repo.

asudoh · 2017-10-12T04:08:31Z

src/components/dropdown/dropdown.js

@@ -4,6 +4,8 @@ import initComponentBySearch from '../../globals/js/mixins/init-component-by-sea
 import trackBlur from '../../globals/js/mixins/track-blur';
 import eventMatches from '../../globals/js/misc/event-matches';
 import on from '../../globals/js/misc/on';
+// import ibmMotion from '../../globals/js/misc/motion-generator';


Please clean-up commented-out code.

tw15egan · 2017-10-13T15:55:12Z

@asudoh for now, we've decided we're going to adapt the static, css motion changes to the library. Animations that require dynamic motion will eventually be added. @wonilsuh has created the @ibm/motion package that will help calculate easing based on the height of an element.

I was thinking that we could eventually just import this package as a dependency, and have the updated, dynamic motion bind to specific components via a data-attribute. What are your thoughts?

asudoh · 2017-10-13T16:11:34Z

Hi @tw15egan incorporating CSS-only motion changes totally makes sense as our short-term strategy. For long-term, we need to see if we can come up with a simple solution that doesn’t affect the codebase much, especially wrt its complexity that I think it’s your concern, too.

I’m actually working with @wonilsuh on this (not to force squeezing it in but to see if we can come up with a solution our team can agree on), and what I have got so far is https://github.com/carbon-design-system/carbon-components/compare/master...asudoh:collapse?w=1 - Which introduces collapsible-element.js that does the most of expand/collapse animation and minimizing the impact to dropdown.js and floating-menu.js. It still impacts both JS files, though, because the open/shown states of those need to be applied after animation ends. I wouldn’t say such added complexity is non-trivial, so would like to have your team’s thoughts on it. e.g. Whether we should make this behavior optional, etc.

wonilsuh · 2017-10-13T23:03:53Z

Splitting this PR into smaller ones. Thank you guys.

* update README and CONTRIBUTING to reflect a11y changes. * lost some text

…-system#354 (carbon-design-system#355)

this file is to replace the existing app icon template—deleted in this PR carbon-design-system/carbon-elements#353

Shixie Shi Trofimov and others added 25 commits September 26, 2017 20:35

button update duo curve 100ms

4a57a94

button curve+duration

853e7af

feat(motion-generator): added system 360 motion generator function

c907123

style(Dropdown): added system 360 motion (curve and dynamic duration)

1c855ed

Merge pull request #1 from wonilsuh/button-motion

65c3b92

Button motion

Merge branch 'wonil-master' of https://github.com/wonilsuh/carbon-com…

898e4c2

…ponents into wonil-master

Merge branch 'master' of github.com:wonilsuh/carbon-components

b1e2c17

style(dropdown): focus border fix

ead6aa8

style(global): duo easing on button toggle

59bdd47

feat(motion): now uses @ibm/motion package

de36295

style(motion): switch to @ibm/motion

bb23d5f

Merge pull request #2 from wonilsuh/easing-curves

7c0ee82

Easing curves

Merge branch 'wonil-master' of github.com:wonilsuh/carbon-components …

ff89edd

…into wonil-master

style(overflow): wip for motion

3146a8a

style(dropdown): motion is fully dynamic and more agnostic

2f73c1b

Merge branch 'wonil-master' into overflow-updates

417324e

fix(floating-menu): focus blur issue fixed

a086bc1

fix(floating-menu): height check now checks the computed height

53f106b

style(floating-menu): dynamic duration with size

47899a0

Merge remote-tracking branch 'upstream/master'

d4df8f2

fix(dropdown): removed console log

4687280

chore(merge): Merge branch 'master' into system-360-master

a56b8fb

fix(style): button easing fix

a296e94

fix(style): added button active states

08d66b0

Merge pull request #4 from wonilsuh/button-active-states

9476b66

fix(style): added button active states

Wonil Suh and others added 3 commits October 10, 2017 09:52

fix(dropdown): PR fixes in progress

396a0e2

fix(dropdown): iteration based method works

2901d36

fix(style): added ease-in-out var

7a26619

asudoh reviewed Oct 11, 2017

View reviewed changes

wonilsuh closed this Oct 11, 2017

wonilsuh reopened this Oct 11, 2017

wonilsuh closed this Oct 11, 2017

wonilsuh reopened this Oct 11, 2017

wonilsuh closed this Oct 11, 2017

wonilsuh reopened this Oct 11, 2017

fix(floating-menu): unit test fix

2b2718b

fix(floatingMenu): console commands removed

9df17b9

wonilsuh closed this Oct 12, 2017

wonilsuh reopened this Oct 12, 2017

wonilsuh closed this Oct 12, 2017

wonilsuh reopened this Oct 12, 2017

Wonil Suh added 2 commits October 11, 2017 22:36

fix(floating-menu): optimized code

499a5b4

fix(floating-menu): addressing unit test issue

640b4b6

asudoh suggested changes Oct 12, 2017

View reviewed changes

Wonil Suh added 4 commits October 12, 2017 00:42

fix(floating-menu): race condition resolved

e5cc87c

fix(floating-menu): using ._place for opening sync layout

f2a97a2

fix(floating-menu): lint

6bc8357

fix(floating-menu): unit test address

0007943

marijohannessen added the duo label Oct 12, 2017

Merge branch 'master' into system-360-master

dd74061

wonilsuh closed this Oct 13, 2017

joshblack pushed a commit to joshblack/carbon that referenced this pull request May 2, 2019

Contributing a11y (carbon-design-system#354)

2ed939f

* update README and CONTRIBUTING to reflect a11y changes. * lost some text

joshblack pushed a commit to joshblack/carbon that referenced this pull request May 2, 2019

fix: react warning in ComposedModal component, resolves carbon-design…

c7e5bcb

…-system#354 (carbon-design-system#355)

joshblack pushed a commit to joshblack/carbon that referenced this pull request May 8, 2019

chore: update app icon template (carbon-design-system#354)

8ff1915

this file is to replace the existing app icon template—deleted in this PR carbon-design-system/carbon-elements#353

snyk-bot mentioned this pull request Sep 16, 2021

[Snyk] Fix for 1 vulnerabilities XirdigH/carbon#62

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

System 360 master #354

System 360 master #354

wonilsuh commented Oct 9, 2017

tay-aitken commented Oct 10, 2017 •

edited

Loading

asudoh left a comment •

edited

Loading

asudoh Oct 11, 2017

wonilsuh Oct 11, 2017 •

edited

Loading

asudoh Oct 11, 2017

asudoh Oct 11, 2017

asudoh Oct 11, 2017

asudoh Oct 11, 2017

asudoh commented Oct 12, 2017

asudoh Oct 12, 2017

asudoh Oct 12, 2017

tw15egan commented Oct 13, 2017

asudoh commented Oct 13, 2017 •

edited

Loading

wonilsuh commented Oct 13, 2017

System 360 master #354

System 360 master #354

Conversation

wonilsuh commented Oct 9, 2017

Overview

Global motion variables

@ibm/motion package

Button

Dropdown

Overflow Menu / Floating Menu

Toggle

tay-aitken commented Oct 10, 2017 • edited Loading

asudoh left a comment • edited Loading

Choose a reason for hiding this comment

asudoh Oct 11, 2017

Choose a reason for hiding this comment

wonilsuh Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

asudoh Oct 11, 2017

Choose a reason for hiding this comment

asudoh Oct 11, 2017

Choose a reason for hiding this comment

asudoh Oct 11, 2017

Choose a reason for hiding this comment

asudoh Oct 11, 2017

Choose a reason for hiding this comment

asudoh commented Oct 12, 2017

asudoh Oct 12, 2017

Choose a reason for hiding this comment

asudoh Oct 12, 2017

Choose a reason for hiding this comment

tw15egan commented Oct 13, 2017

asudoh commented Oct 13, 2017 • edited Loading

wonilsuh commented Oct 13, 2017

`@ibm/motion` package

tay-aitken commented Oct 10, 2017 •

edited

Loading

asudoh left a comment •

edited

Loading

wonilsuh Oct 11, 2017 •

edited

Loading

asudoh commented Oct 13, 2017 •

edited

Loading