From 7c8b4aaea46f0aa41f31f0e94876e7c1599db97e Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Mon, 29 Apr 2019 14:20:55 -0700 Subject: [PATCH 01/17] chore: add develop to travis.yml (#4659) --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 3d376e1155a..8f0db81f75a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ branches: # This prevents excessive resource usage and CI slowness. only: - master + - develop before_install: # Source the scripts to export their env vars. See https://superuser.com/a/176788/62792 From 7c78d3a444b651e194a4f5b718e7d2392a2c7916 Mon Sep 17 00:00:00 2001 From: Patty RoDee Date: Tue, 30 Apr 2019 11:33:20 -0700 Subject: [PATCH 02/17] feat(chips): Use semantic button elements (#4627) Remove the top and bottom padding from the components. Remove the top and bottom margins from the button content. Button content should be center aligned. Use a ... ``` @@ -83,10 +83,10 @@ However, you can also use SVG, [Font Awesome](https://fontawesome.com/), or any #### Leading icon ```html -
+
+ Add to calendar + ``` #### Trailing icon @@ -94,10 +94,10 @@ However, you can also use SVG, [Font Awesome](https://fontawesome.com/), or any A trailing icon comes with the functionality to remove the chip from the set. If you're adding a trailing icon, also set `tabindex="0"` and `role="button"` to make it accessible by keyboard and screenreader. Trailing icons should only be added to [input chips](#input-chips). ```html -
-
Jane Smith
+
+ ``` ### Choice Chips @@ -116,15 +116,15 @@ Filter chips are a variant of chips which allow multiple selection from a set of ```html
-
-
+
-
Filterable content
-
+ + Filterable content + ...
``` @@ -133,16 +133,16 @@ To use a leading icon in a filter chip, put the `mdc-chip__icon--leading` elemen ```html
-
+
+ + Filterable content + ...
``` @@ -194,24 +194,24 @@ chipSet.listen('MDCChip:removal', function(event) { To display a pre-selected filter or choice chip, add the class `mdc-chip--selected` to the root chip element. ```html -
-
Add to calendar
-
+ ``` To pre-select filter chips that have a leading icon, also add the class `mdc-chip__icon--leading-hidden` to the `mdc-chip__icon--leading` element. This will ensure that the checkmark displaces the leading icon. ```html -
+
+ + Filterable content + ``` ## Style Customization @@ -257,8 +257,8 @@ Mixin | Description `mdc-chip-trailing-icon-color($color, $opacity, $hover-opacity, $focus-opacity)` | Customizes the color of a trailing icon in a chip, optionally customizes regular/hover/focus opacities `mdc-chip-leading-icon-size($size)` | Customizes the size of a leading icon in a chip `mdc-chip-trailing-icon-size($size)` | Customizes the size of a trailing icon in a chip -`mdc-chip-leading-icon-margin($top, $right, $bottom, $left)` | Customizes the margin of a leading icon in a chip -`mdc-chip-trailing-icon-margin($top, $right, $bottom, $left)` | Customizes the margin of a trailing icon in a chip +`mdc-chip-leading-icon-margin($left-margin, $right-margin)` | Customizes the margin of a leading icon in a chip +`mdc-chip-trailing-icon-margin($left-margin, $right-margin)` | Customizes the margin of a trailing icon in a chip `mdc-chip-elevation-transition()` | Adds a MDC elevation transition to the chip. This should be used instead of setting transition with `mdc-elevation-transition-value()` directly when a box shadow transition is desired for a chip > _NOTE_: `mdc-chip-set-spacing` also sets the amount of space between a chip and the edge of the set it's contained in. diff --git a/packages/mdc-chips/_mixins.scss b/packages/mdc-chips/_mixins.scss index 8ccfe68c793..7d9c0b55dd8 100644 --- a/packages/mdc-chips/_mixins.scss +++ b/packages/mdc-chips/_mixins.scss @@ -100,14 +100,15 @@ @mixin mdc-chip-outline-width($width, $horizontal-padding: $mdc-chip-horizontal-padding) { // Note: Adjust padding to maintain consistent width with non-outlined chips $horizontal-padding-value: max($horizontal-padding - $width, 0); - $vertical-padding-value: max($mdc-chip-vertical-padding - $width, 0); - padding: $vertical-padding-value $horizontal-padding-value; + padding-right: $horizontal-padding-value; + padding-left: $horizontal-padding-value; border-width: $width; } @mixin mdc-chip-horizontal-padding($padding) { - padding: $mdc-chip-vertical-padding $padding; + padding-right: $padding; + padding-left: $padding; } @mixin mdc-chip-height($height) { @@ -169,26 +170,20 @@ } @mixin mdc-chip-leading-icon-margin( - $top: $mdc-chip-leading-icon-margin-top, - $right: $mdc-chip-leading-icon-margin-right, - $bottom: $mdc-chip-leading-icon-margin-bottom, - $left: $mdc-chip-leading-icon-margin-left) { + $left-margin: $mdc-chip-leading-icon-margin-left, + $right-margin: $mdc-chip-leading-icon-margin-right) { &.mdc-chip--selected .mdc-chip__checkmark, .mdc-chip__icon--leading:not(.mdc-chip__icon--leading-hidden) { - @include mdc-rtl-reflexive-property(margin, $left, $right); - - margin-top: $top; - margin-bottom: $bottom; + @include mdc-rtl-reflexive-property(margin, $left-margin, $right-margin); } } @mixin mdc-chip-trailing-icon-margin( - $top: $mdc-chip-trailing-icon-margin-top, - $right: $mdc-chip-trailing-icon-margin-right, - $bottom: $mdc-chip-trailing-icon-margin-bottom, - $left: $mdc-chip-trailing-icon-margin-left) { + $left-margin: $mdc-chip-trailing-icon-margin-left, + $right-margin: $mdc-chip-trailing-icon-margin-right) { .mdc-chip__icon--trailing { - margin: $top $right $bottom $left; + margin-right: $right-margin; + margin-left: $left-margin; } } diff --git a/packages/mdc-chips/_variables.scss b/packages/mdc-chips/_variables.scss index 2dc42c7ebfc..6960df46903 100644 --- a/packages/mdc-chips/_variables.scss +++ b/packages/mdc-chips/_variables.scss @@ -26,7 +26,6 @@ $mdc-chip-fill-color-default: mix(mdc-theme-prop-value(on-surface), mdc-theme-prop-value(surface), 12%) !default; $mdc-chip-ink-color-default: rgba(mdc-theme-prop-value(on-surface), .87) !default; $mdc-chip-horizontal-padding: 12px !default; -$mdc-chip-vertical-padding: 7px !default; $mdc-chip-height-default: 32px !default; $mdc-chip-icon-color: mdc-theme-prop-value(on-surface) !default; @@ -44,14 +43,10 @@ $mdc-chip-checkmark-animation-duration: 150ms !default; $mdc-chip-width-animation-duration: 150ms !default; $mdc-chip-opacity-animation-duration: 75ms !default; -$mdc-chip-leading-icon-margin-top: -4px !default; $mdc-chip-leading-icon-margin-right: 4px !default; -$mdc-chip-leading-icon-margin-bottom: -4px !default; $mdc-chip-leading-icon-margin-left: -4px !default; -$mdc-chip-trailing-icon-margin-top: 0 !default; $mdc-chip-trailing-icon-margin-right: -4px !default; -$mdc-chip-trailing-icon-margin-bottom: 0 !default; $mdc-chip-trailing-icon-margin-left: 4px !default; $mdc-chip-exit-transition: diff --git a/packages/mdc-chips/chip/mdc-chip.scss b/packages/mdc-chips/chip/mdc-chip.scss index 2e553ba3310..631ce9b0421 100644 --- a/packages/mdc-chips/chip/mdc-chip.scss +++ b/packages/mdc-chips/chip/mdc-chip.scss @@ -43,12 +43,18 @@ display: inline-flex; position: relative; align-items: center; - box-sizing: border-box; - padding: $mdc-chip-vertical-padding $mdc-chip-horizontal-padding; + padding: 0 $mdc-chip-horizontal-padding; + border-width: 0; outline: none; cursor: pointer; + -webkit-appearance: none; overflow: hidden; + &::-moz-focus-inner { + padding: 0; + border: 0; + } + &:hover { @include mdc-theme-prop(color, on-surface); } diff --git a/test/screenshot/golden.json b/test/screenshot/golden.json index 0a6944760ea..11975bcae9c 100644 --- a/test/screenshot/golden.json +++ b/test/screenshot/golden.json @@ -240,11 +240,11 @@ } }, "spec/mdc-chips/mixins/chip-set-spacing.html": { - "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/10/19/20_00_07_456/spec/mdc-chips/mixins/chip-set-spacing.html?utm_source=golden_json", + "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/30/17_01_54_496/spec/mdc-chips/mixins/chip-set-spacing.html?utm_source=golden_json", "screenshots": { - "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/10/19/20_00_07_456/spec/mdc-chips/mixins/chip-set-spacing.html.windows_chrome_69.png", - "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/rlfriedman/2018/09/13/20_54_11_470/spec/mdc-chips/mixins/chip-set-spacing.html.windows_firefox_62.png", - "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/rlfriedman/2018/09/13/20_54_11_470/spec/mdc-chips/mixins/chip-set-spacing.html.windows_ie_11.png" + "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/30/17_01_54_496/spec/mdc-chips/mixins/chip-set-spacing.html.windows_chrome_73.png", + "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/30/17_01_54_496/spec/mdc-chips/mixins/chip-set-spacing.html.windows_firefox_65.png", + "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/30/17_01_54_496/spec/mdc-chips/mixins/chip-set-spacing.html.windows_ie_11.png" } }, "spec/mdc-chips/mixins/fill-color-accessible.html": { diff --git a/test/screenshot/spec/mdc-chips/classes/action.html b/test/screenshot/spec/mdc-chips/classes/action.html index d05abe20c85..f1217c0b289 100644 --- a/test/screenshot/spec/mdc-chips/classes/action.html +++ b/test/screenshot/spec/mdc-chips/classes/action.html @@ -44,39 +44,39 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/classes/choice.html b/test/screenshot/spec/mdc-chips/classes/choice.html index a35e97fbbc9..3322dd10d09 100644 --- a/test/screenshot/spec/mdc-chips/classes/choice.html +++ b/test/screenshot/spec/mdc-chips/classes/choice.html @@ -44,35 +44,35 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
diff --git a/test/screenshot/spec/mdc-chips/classes/filter.html b/test/screenshot/spec/mdc-chips/classes/filter.html index 412b0aebfe3..836e56d6ba3 100644 --- a/test/screenshot/spec/mdc-chips/classes/filter.html +++ b/test/screenshot/spec/mdc-chips/classes/filter.html @@ -44,137 +44,137 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/classes/input.html b/test/screenshot/spec/mdc-chips/classes/input.html index 664091d4021..65f9e479a90 100644 --- a/test/screenshot/spec/mdc-chips/classes/input.html +++ b/test/screenshot/spec/mdc-chips/classes/input.html @@ -44,65 +44,65 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
Chip One
+
-
-
Chip Two
+ +
-
-
Chip Three
+ +
-
-
Chip Four
+ +
+
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/fixture.scss b/test/screenshot/spec/mdc-chips/fixture.scss index d6b59d5367d..c35772fb5c0 100644 --- a/test/screenshot/spec/mdc-chips/fixture.scss +++ b/test/screenshot/spec/mdc-chips/fixture.scss @@ -35,6 +35,14 @@ $custom-chip-ink-color: $material-color-blue-500; .custom-chip--chip-set-spacing { @include mdc-chip-set-spacing(20px); + + @at-root { + .custom-chip-test-cell { + @include test-cell-size(321, 124); + + padding: 0; + } + } } .custom-chip--shape-radius { @@ -82,11 +90,11 @@ $custom-chip-ink-color: $material-color-blue-500; } .custom-chip--leading-icon-margin { - @include mdc-chip-leading-icon-margin(5px, 5px, 5px, 5px); + @include mdc-chip-leading-icon-margin(5px, 5px); } .custom-chip--trailing-icon-margin { - @include mdc-chip-trailing-icon-margin(5px, 5px, 5px, 5px); + @include mdc-chip-trailing-icon-margin(5px, 5px); } .custom-chip--horizontal-padding { diff --git a/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html b/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html index e5857d8d298..98ea4bf5889 100644 --- a/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html +++ b/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html @@ -42,103 +42,103 @@
-
-
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+
+
+ + + +
-
-
-
+
+
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
-
-
+
+
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
-
-
+
+
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html b/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html index 2e1a11d0bf5..807096e0574 100644 --- a/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html +++ b/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/fill-color.html b/test/screenshot/spec/mdc-chips/mixins/fill-color.html index a8062e3e50b..6bfded2e83d 100644 --- a/test/screenshot/spec/mdc-chips/mixins/fill-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/fill-color.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/height.html b/test/screenshot/spec/mdc-chips/mixins/height.html index 35099e882e8..3482a30c5ac 100644 --- a/test/screenshot/spec/mdc-chips/mixins/height.html +++ b/test/screenshot/spec/mdc-chips/mixins/height.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html b/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html index 83fa4f66c05..78c96e28eb7 100644 --- a/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html +++ b/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/ink-color.html b/test/screenshot/spec/mdc-chips/mixins/ink-color.html index c9b4e96d00b..9077cde1c0f 100644 --- a/test/screenshot/spec/mdc-chips/mixins/ink-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/ink-color.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html b/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html index 73822cd4712..150c9454ab5 100644 --- a/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html @@ -44,88 +44,88 @@
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html b/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html index a78c8eaf135..ce9b00ab87e 100644 --- a/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html +++ b/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html @@ -44,166 +44,166 @@
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html b/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html index 9ab3ca0c3fa..8f3c13f568e 100644 --- a/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html +++ b/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html @@ -44,88 +44,88 @@
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/outline.html b/test/screenshot/spec/mdc-chips/mixins/outline.html index f97829649cc..3148aedd5c2 100644 --- a/test/screenshot/spec/mdc-chips/mixins/outline.html +++ b/test/screenshot/spec/mdc-chips/mixins/outline.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html b/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html index 322086eb923..8bbbbd3f894 100644 --- a/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html @@ -44,55 +44,55 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/mixins/shape-radius.html b/test/screenshot/spec/mdc-chips/mixins/shape-radius.html index 1c03380cfed..fa018867a2f 100644 --- a/test/screenshot/spec/mdc-chips/mixins/shape-radius.html +++ b/test/screenshot/spec/mdc-chips/mixins/shape-radius.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html index 8d6f219d114..c24faad3822 100644 --- a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html @@ -44,26 +44,26 @@
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html index 69cf7d880ff..e8ca1c1f8f8 100644 --- a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html +++ b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html @@ -44,26 +44,26 @@
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html index 205b282b3ba..180b5690ad9 100644 --- a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html +++ b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html @@ -44,26 +44,26 @@
-
+
-
+ +
-
+ +
-
+ +
+
From dd54c483c71a040a13e51da06f2260995682216a Mon Sep 17 00:00:00 2001 From: Joy Zhong Date: Thu, 2 May 2019 10:17:21 -0400 Subject: [PATCH 03/17] refactor(dialog): Split dialog Foundation#handleInteraction into #handleClick/#handleKeydown. (#4655) BREAKING CHANGE: Dialog `Foundation#handleInteraction` has been split into two methods: `#handleClick` and `#handleKeydown`. --- packages/mdc-dialog/README.md | 8 ++-- packages/mdc-dialog/component.ts | 14 +++--- packages/mdc-dialog/foundation.ts | 35 +++++++++----- test/unit/mdc-dialog/foundation.test.js | 61 +++++++++++++------------ test/unit/mdc-dialog/mdc-dialog.test.js | 8 ++-- 5 files changed, 74 insertions(+), 52 deletions(-) diff --git a/packages/mdc-dialog/README.md b/packages/mdc-dialog/README.md index 392428f801f..fb8d7f74f28 100644 --- a/packages/mdc-dialog/README.md +++ b/packages/mdc-dialog/README.md @@ -400,8 +400,9 @@ Method Signature | Description `setScrimClickAction(action: string)` | Sets the action reflected when the scrim is clicked. Setting to `''` disables closing the dialog via scrim click. `getAutoStackButtons() => boolean` | Returns whether stacked/unstacked action button layout is automatically handled during layout logic. `setAutoStackButtons(autoStack: boolean) => void` | Sets whether stacked/unstacked action button layout is automatically handled during layout logic. -`handleInteraction(event: Event)` | Handles `click` and `keydown` events on or within the dialog's root element -`handleDocumentKeydown(event: Event)` | Handles `keydown` events on or within the document while the dialog is open +`handleClick(event: MouseEvent)` | Handles `click` events on or within the dialog's root element. +`handleKeydown(event: KeyboardEvent)` | Handles `keydown` events on or within the dialog's root element. +`handleDocumentKeydown(event: Event)` | Handles `keydown` events on or within the document while the dialog is open. #### Event Handlers @@ -409,7 +410,8 @@ When wrapping the Dialog foundation, the following events must be bound to the i Event | Target | Foundation Handler | Register | Deregister --- | --- | --- | --- | --- -`click` | `.mdc-dialog` (root) | `handleInteraction` | During initialization | During destruction +`click` | `.mdc-dialog` (root) | `handleClick` | During initialization | During destruction +`keydown` | `.mdc-dialog` (root) | `handleKeydown` | During initialization | During destruction `keydown` | `document` | `handleDocumentKeydown` | On `MDCDialog:opening` | On `MDCDialog:closing` `resize` | `window` | `layout` | On `MDCDialog:opening` | On `MDCDialog:closing` `orientationchange` | `window` | `layout` | On `MDCDialog:opening` | On `MDCDialog:closing` diff --git a/packages/mdc-dialog/component.ts b/packages/mdc-dialog/component.ts index 675cab11164..ff90dab50cc 100644 --- a/packages/mdc-dialog/component.ts +++ b/packages/mdc-dialog/component.ts @@ -77,7 +77,8 @@ export class MDCDialog extends MDCComponent { private focusTrap_!: FocusTrap; // assigned in initialSyncWithDOM() private focusTrapFactory_?: MDCDialogFocusTrapFactory; // assigned in initialize() - private handleInteraction_!: SpecificEventListener<'click' | 'keydown'>; // assigned in initialSyncWithDOM() + private handleClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() + private handleKeydown_!: SpecificEventListener<'keydown'>; // assigned in initialSyncWithDOM() private handleDocumentKeydown_!: SpecificEventListener<'keydown'>; // assigned in initialSyncWithDOM() private handleLayout_!: EventListener; // assigned in initialSyncWithDOM() private handleOpening_!: EventListener; // assigned in initialSyncWithDOM() @@ -107,7 +108,8 @@ export class MDCDialog extends MDCComponent { initialSyncWithDOM() { this.focusTrap_ = util.createFocusTrapInstance(this.container_, this.focusTrapFactory_, this.initialFocusEl_); - this.handleInteraction_ = this.foundation_.handleInteraction.bind(this.foundation_); + this.handleClick_ = this.foundation_.handleClick.bind(this.foundation_); + this.handleKeydown_ = this.foundation_.handleKeydown.bind(this.foundation_); this.handleDocumentKeydown_ = this.foundation_.handleDocumentKeydown.bind(this.foundation_); this.handleLayout_ = this.layout.bind(this); @@ -121,15 +123,15 @@ export class MDCDialog extends MDCComponent { document.removeEventListener('keydown', this.handleDocumentKeydown_); }; - this.listen('click', this.handleInteraction_); - this.listen('keydown', this.handleInteraction_); + this.listen('click', this.handleClick_); + this.listen('keydown', this.handleKeydown_); this.listen(strings.OPENING_EVENT, this.handleOpening_); this.listen(strings.CLOSING_EVENT, this.handleClosing_); } destroy() { - this.unlisten('click', this.handleInteraction_); - this.unlisten('keydown', this.handleInteraction_); + this.unlisten('click', this.handleClick_); + this.unlisten('keydown', this.handleKeydown_); this.unlisten(strings.OPENING_EVENT, this.handleOpening_); this.unlisten(strings.CLOSING_EVENT, this.handleClosing_); this.handleClosing_(); diff --git a/packages/mdc-dialog/foundation.ts b/packages/mdc-dialog/foundation.ts index 76a69f17c7c..3b259b097b5 100644 --- a/packages/mdc-dialog/foundation.ts +++ b/packages/mdc-dialog/foundation.ts @@ -176,26 +176,39 @@ export class MDCDialogFoundation extends MDCFoundation { }); } - handleInteraction(evt: MouseEvent | KeyboardEvent) { - const isClick = evt.type === 'click'; - const isEnter = (evt as KeyboardEvent).key === 'Enter' || (evt as KeyboardEvent).keyCode === 13; - const isSpace = (evt as KeyboardEvent).key === 'Space' || (evt as KeyboardEvent).keyCode === 32; + /** Closes the dialog if scrim or action button click. */ + handleClick(evt: MouseEvent) { const isScrim = this.adapter_.eventTargetMatches(evt.target, strings.SCRIM_SELECTOR); - const isDefault = !this.adapter_.eventTargetMatches(evt.target, strings.SUPPRESS_DEFAULT_PRESS_SELECTOR); - - // Check for scrim click first since it doesn't require querying ancestors - if (isClick && isScrim && this.scrimClickAction_ !== '') { + // Check for scrim click first since it doesn't require querying ancestors. + if (isScrim && this.scrimClickAction_ !== '') { this.close(this.scrimClickAction_); - } else if (isClick || isSpace || isEnter) { + } else { const action = this.adapter_.getActionFromEvent(evt); if (action) { this.close(action); - } else if (isEnter && isDefault) { - this.adapter_.clickDefaultButton(); } } } + handleKeydown(evt: KeyboardEvent) { + const isEnter = evt.key === 'Enter' || evt.keyCode === 13; + if (!isEnter) { + return; + } + const action = this.adapter_.getActionFromEvent(evt); + if (action) { + // Action button callback is handled in `handleClick`, + // since space/enter keydowns on buttons trigger click events. + return; + } + + const isDefault = !this.adapter_.eventTargetMatches( + evt.target, strings.SUPPRESS_DEFAULT_PRESS_SELECTOR); + if (isEnter && isDefault) { + this.adapter_.clickDefaultButton(); + } + } + handleDocumentKeydown(evt: KeyboardEvent) { const isEscape = evt.key === 'Escape' || evt.keyCode === 27; if (isEscape && this.escapeKeyAction_ !== '') { diff --git a/test/unit/mdc-dialog/foundation.test.js b/test/unit/mdc-dialog/foundation.test.js index 56040beae8e..a38c12d638f 100644 --- a/test/unit/mdc-dialog/foundation.test.js +++ b/test/unit/mdc-dialog/foundation.test.js @@ -36,12 +36,6 @@ const ENTER_EVENTS = [ {type: 'keydown', keyCode: 13, target: {}}, ]; -const INTERACTION_EVENTS = [ - {type: 'click', target: {}}, - {type: 'keydown', key: 'Space', target: {}}, - {type: 'keydown', keyCode: 32, target: {}}, -].concat(ENTER_EVENTS); - suite('MDCDialogFoundation'); test('exports cssClasses', () => { @@ -359,82 +353,93 @@ test('#layout removes scrollable class when content is not scrollable', () => { td.verify(mockAdapter.removeClass(cssClasses.SCROLLABLE)); }); -test(`interaction closes dialog when ${strings.ACTION_ATTRIBUTE} attribute is present`, () => { +test(`#handleClick: Click closes dialog when ${strings.ACTION_ATTRIBUTE} attribute is present`, () => { const {foundation, mockAdapter} = setupTest(); const action = 'action'; foundation.close = td.func('close'); - INTERACTION_EVENTS.forEach((event) => { - td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action); - foundation.open(); - foundation.handleInteraction(event); + const event = {target: {}}; + td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action); + foundation.open(); + foundation.handleClick(event); - td.verify(foundation.close(action)); - td.reset(); - }); + td.verify(foundation.close(action)); }); -test('interaction does not close dialog with action for non-activation keys', () => { +test('#handleKeydown: Keydown does not close dialog with action for non-activation keys', () => { const {foundation, mockAdapter} = setupTest(); const action = 'action'; - const event = {type: 'keydown', key: 'Escape', target: {}}; + const event = {type: 'keydown', key: 'Shift', target: {}}; foundation.close = td.func('close'); td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action); foundation.open(); - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(foundation.close(action), {times: 0}); }); -test(`interaction does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => { +test(`#handleClick: Click does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => { const {foundation, mockAdapter} = setupTest(); foundation.close = td.func('close'); - INTERACTION_EVENTS.forEach((event) => { + const event = {target: {}}; + td.when(mockAdapter.getActionFromEvent(event)).thenReturn(''); + foundation.open(); + foundation.handleClick(event); + + td.verify(foundation.close(td.matchers.isA(String)), {times: 0}); +}); + +test(`#handleKeydown: Keydown does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => { + const {foundation, mockAdapter} = setupTest(); + foundation.close = td.func('close'); + + ENTER_EVENTS.forEach((event) => { td.when(mockAdapter.getActionFromEvent(event)).thenReturn(''); foundation.open(); - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(foundation.close(td.matchers.isA(String)), {times: 0}); td.reset(); }); }); -test('enter keydown calls adapter.clickDefaultButton', () => { +test('#handleKeydown: Enter keydown calls adapter.clickDefaultButton', () => { const {foundation, mockAdapter} = setupTest(); ENTER_EVENTS.forEach((event) => { - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(mockAdapter.clickDefaultButton()); td.reset(); }); }); -test('enter keydown does not call adapter.clickDefaultButton when it should be suppressed', () => { +test('#handleKeydown: Enter keydown does not call adapter.clickDefaultButton when it should be suppressed', () => { const {foundation, mockAdapter} = setupTest(); ENTER_EVENTS.forEach((event) => { td.when(mockAdapter.eventTargetMatches(event.target, strings.SUPPRESS_DEFAULT_PRESS_SELECTOR)).thenReturn(true); - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(mockAdapter.clickDefaultButton(), {times: 0}); td.reset(); }); }); -test(`click closes dialog when ${strings.SCRIM_SELECTOR} selector matches`, () => { +test(`#handleClick: Click closes dialog when ${strings.SCRIM_SELECTOR} selector matches`, () => { const {foundation, mockAdapter} = setupTest(); const evt = {type: 'click', target: {}}; foundation.close = td.func('close'); td.when(mockAdapter.eventTargetMatches(evt.target, strings.SCRIM_SELECTOR)).thenReturn(true); foundation.open(); - foundation.handleInteraction(evt); + foundation.handleClick(evt); td.verify(foundation.close(foundation.getScrimClickAction())); }); -test(`click does nothing when ${strings.SCRIM_SELECTOR} class is present but scrimClickAction is empty string`, () => { +test(`#handleClick: Click does nothing when ${strings.SCRIM_SELECTOR} class is present but scrimClickAction is + empty string`, () => { const {foundation, mockAdapter} = setupTest(); const evt = {type: 'click', target: {}}; foundation.close = td.func('close'); @@ -442,7 +447,7 @@ test(`click does nothing when ${strings.SCRIM_SELECTOR} class is present but scr foundation.setScrimClickAction(''); foundation.open(); - foundation.handleInteraction(evt); + foundation.handleClick(evt); td.verify(foundation.close(td.matchers.isA(String)), {times: 0}); }); diff --git a/test/unit/mdc-dialog/mdc-dialog.test.js b/test/unit/mdc-dialog/mdc-dialog.test.js index 4dcd9821ac4..47a0287f883 100644 --- a/test/unit/mdc-dialog/mdc-dialog.test.js +++ b/test/unit/mdc-dialog/mdc-dialog.test.js @@ -108,14 +108,14 @@ test('attachTo throws an error when container element is missing', () => { test('#initialSyncWithDOM registers click handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); domEvents.emit(root, 'click'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 1}); + td.verify(mockFoundation.handleClick(td.matchers.isA(Event)), {times: 1}); component.destroy(); }); test('#initialSyncWithDOM registers keydown handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); domEvents.emit(root, 'keydown'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 1}); + td.verify(mockFoundation.handleKeydown(td.matchers.isA(Event)), {times: 1}); component.destroy(); }); @@ -123,14 +123,14 @@ test('#destroy deregisters click handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); component.destroy(); domEvents.emit(root, 'click'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 0}); + td.verify(mockFoundation.handleClick(td.matchers.isA(Event)), {times: 0}); }); test('#destroy deregisters keydown handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); component.destroy(); domEvents.emit(root, 'keydown'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 0}); + td.verify(mockFoundation.handleKeydown(td.matchers.isA(Event)), {times: 0}); }); test(`${strings.OPENING_EVENT} registers document keydown handler and ${strings.CLOSING_EVENT} deregisters it`, () => { From 8b704aacb41f50c78aecacd8b1601a29fb290ac2 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Mon, 29 Apr 2019 14:20:55 -0700 Subject: [PATCH 04/17] chore: add develop to travis.yml (#4659) --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 3d376e1155a..8f0db81f75a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ branches: # This prevents excessive resource usage and CI slowness. only: - master + - develop before_install: # Source the scripts to export their env vars. See https://superuser.com/a/176788/62792 From fac44b120cf673ded418261dcc830d34542c0ed6 Mon Sep 17 00:00:00 2001 From: Patty RoDee Date: Tue, 30 Apr 2019 11:33:20 -0700 Subject: [PATCH 05/17] feat(chips): Use semantic button elements (#4627) Remove the top and bottom padding from the components. Remove the top and bottom margins from the button content. Button content should be center aligned. Use a ...
``` @@ -83,10 +83,10 @@ However, you can also use SVG, [Font Awesome](https://fontawesome.com/), or any #### Leading icon ```html -
+
+ Add to calendar + ``` #### Trailing icon @@ -94,10 +94,10 @@ However, you can also use SVG, [Font Awesome](https://fontawesome.com/), or any A trailing icon comes with the functionality to remove the chip from the set. If you're adding a trailing icon, also set `tabindex="0"` and `role="button"` to make it accessible by keyboard and screenreader. Trailing icons should only be added to [input chips](#input-chips). ```html -
-
Jane Smith
+
+ ``` ### Choice Chips @@ -116,15 +116,15 @@ Filter chips are a variant of chips which allow multiple selection from a set of ```html
-
-
+
-
Filterable content
-
+ + Filterable content + ...
``` @@ -133,16 +133,16 @@ To use a leading icon in a filter chip, put the `mdc-chip__icon--leading` elemen ```html
-
+
+ + Filterable content + ...
``` @@ -194,24 +194,24 @@ chipSet.listen('MDCChip:removal', function(event) { To display a pre-selected filter or choice chip, add the class `mdc-chip--selected` to the root chip element. ```html -
-
Add to calendar
-
+ ``` To pre-select filter chips that have a leading icon, also add the class `mdc-chip__icon--leading-hidden` to the `mdc-chip__icon--leading` element. This will ensure that the checkmark displaces the leading icon. ```html -
+
+ + Filterable content + ``` ## Style Customization @@ -257,8 +257,8 @@ Mixin | Description `mdc-chip-trailing-icon-color($color, $opacity, $hover-opacity, $focus-opacity)` | Customizes the color of a trailing icon in a chip, optionally customizes regular/hover/focus opacities `mdc-chip-leading-icon-size($size)` | Customizes the size of a leading icon in a chip `mdc-chip-trailing-icon-size($size)` | Customizes the size of a trailing icon in a chip -`mdc-chip-leading-icon-margin($top, $right, $bottom, $left)` | Customizes the margin of a leading icon in a chip -`mdc-chip-trailing-icon-margin($top, $right, $bottom, $left)` | Customizes the margin of a trailing icon in a chip +`mdc-chip-leading-icon-margin($left-margin, $right-margin)` | Customizes the margin of a leading icon in a chip +`mdc-chip-trailing-icon-margin($left-margin, $right-margin)` | Customizes the margin of a trailing icon in a chip `mdc-chip-elevation-transition()` | Adds a MDC elevation transition to the chip. This should be used instead of setting transition with `mdc-elevation-transition-value()` directly when a box shadow transition is desired for a chip > _NOTE_: `mdc-chip-set-spacing` also sets the amount of space between a chip and the edge of the set it's contained in. diff --git a/packages/mdc-chips/_mixins.scss b/packages/mdc-chips/_mixins.scss index 8ccfe68c793..7d9c0b55dd8 100644 --- a/packages/mdc-chips/_mixins.scss +++ b/packages/mdc-chips/_mixins.scss @@ -100,14 +100,15 @@ @mixin mdc-chip-outline-width($width, $horizontal-padding: $mdc-chip-horizontal-padding) { // Note: Adjust padding to maintain consistent width with non-outlined chips $horizontal-padding-value: max($horizontal-padding - $width, 0); - $vertical-padding-value: max($mdc-chip-vertical-padding - $width, 0); - padding: $vertical-padding-value $horizontal-padding-value; + padding-right: $horizontal-padding-value; + padding-left: $horizontal-padding-value; border-width: $width; } @mixin mdc-chip-horizontal-padding($padding) { - padding: $mdc-chip-vertical-padding $padding; + padding-right: $padding; + padding-left: $padding; } @mixin mdc-chip-height($height) { @@ -169,26 +170,20 @@ } @mixin mdc-chip-leading-icon-margin( - $top: $mdc-chip-leading-icon-margin-top, - $right: $mdc-chip-leading-icon-margin-right, - $bottom: $mdc-chip-leading-icon-margin-bottom, - $left: $mdc-chip-leading-icon-margin-left) { + $left-margin: $mdc-chip-leading-icon-margin-left, + $right-margin: $mdc-chip-leading-icon-margin-right) { &.mdc-chip--selected .mdc-chip__checkmark, .mdc-chip__icon--leading:not(.mdc-chip__icon--leading-hidden) { - @include mdc-rtl-reflexive-property(margin, $left, $right); - - margin-top: $top; - margin-bottom: $bottom; + @include mdc-rtl-reflexive-property(margin, $left-margin, $right-margin); } } @mixin mdc-chip-trailing-icon-margin( - $top: $mdc-chip-trailing-icon-margin-top, - $right: $mdc-chip-trailing-icon-margin-right, - $bottom: $mdc-chip-trailing-icon-margin-bottom, - $left: $mdc-chip-trailing-icon-margin-left) { + $left-margin: $mdc-chip-trailing-icon-margin-left, + $right-margin: $mdc-chip-trailing-icon-margin-right) { .mdc-chip__icon--trailing { - margin: $top $right $bottom $left; + margin-right: $right-margin; + margin-left: $left-margin; } } diff --git a/packages/mdc-chips/_variables.scss b/packages/mdc-chips/_variables.scss index 2dc42c7ebfc..6960df46903 100644 --- a/packages/mdc-chips/_variables.scss +++ b/packages/mdc-chips/_variables.scss @@ -26,7 +26,6 @@ $mdc-chip-fill-color-default: mix(mdc-theme-prop-value(on-surface), mdc-theme-prop-value(surface), 12%) !default; $mdc-chip-ink-color-default: rgba(mdc-theme-prop-value(on-surface), .87) !default; $mdc-chip-horizontal-padding: 12px !default; -$mdc-chip-vertical-padding: 7px !default; $mdc-chip-height-default: 32px !default; $mdc-chip-icon-color: mdc-theme-prop-value(on-surface) !default; @@ -44,14 +43,10 @@ $mdc-chip-checkmark-animation-duration: 150ms !default; $mdc-chip-width-animation-duration: 150ms !default; $mdc-chip-opacity-animation-duration: 75ms !default; -$mdc-chip-leading-icon-margin-top: -4px !default; $mdc-chip-leading-icon-margin-right: 4px !default; -$mdc-chip-leading-icon-margin-bottom: -4px !default; $mdc-chip-leading-icon-margin-left: -4px !default; -$mdc-chip-trailing-icon-margin-top: 0 !default; $mdc-chip-trailing-icon-margin-right: -4px !default; -$mdc-chip-trailing-icon-margin-bottom: 0 !default; $mdc-chip-trailing-icon-margin-left: 4px !default; $mdc-chip-exit-transition: diff --git a/packages/mdc-chips/chip/mdc-chip.scss b/packages/mdc-chips/chip/mdc-chip.scss index 2e553ba3310..631ce9b0421 100644 --- a/packages/mdc-chips/chip/mdc-chip.scss +++ b/packages/mdc-chips/chip/mdc-chip.scss @@ -43,12 +43,18 @@ display: inline-flex; position: relative; align-items: center; - box-sizing: border-box; - padding: $mdc-chip-vertical-padding $mdc-chip-horizontal-padding; + padding: 0 $mdc-chip-horizontal-padding; + border-width: 0; outline: none; cursor: pointer; + -webkit-appearance: none; overflow: hidden; + &::-moz-focus-inner { + padding: 0; + border: 0; + } + &:hover { @include mdc-theme-prop(color, on-surface); } diff --git a/test/screenshot/spec/mdc-chips/classes/action.html b/test/screenshot/spec/mdc-chips/classes/action.html index d05abe20c85..f1217c0b289 100644 --- a/test/screenshot/spec/mdc-chips/classes/action.html +++ b/test/screenshot/spec/mdc-chips/classes/action.html @@ -44,39 +44,39 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/classes/choice.html b/test/screenshot/spec/mdc-chips/classes/choice.html index a35e97fbbc9..3322dd10d09 100644 --- a/test/screenshot/spec/mdc-chips/classes/choice.html +++ b/test/screenshot/spec/mdc-chips/classes/choice.html @@ -44,35 +44,35 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
diff --git a/test/screenshot/spec/mdc-chips/classes/filter.html b/test/screenshot/spec/mdc-chips/classes/filter.html index 412b0aebfe3..836e56d6ba3 100644 --- a/test/screenshot/spec/mdc-chips/classes/filter.html +++ b/test/screenshot/spec/mdc-chips/classes/filter.html @@ -44,137 +44,137 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/classes/input.html b/test/screenshot/spec/mdc-chips/classes/input.html index 664091d4021..65f9e479a90 100644 --- a/test/screenshot/spec/mdc-chips/classes/input.html +++ b/test/screenshot/spec/mdc-chips/classes/input.html @@ -44,65 +44,65 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
Chip One
+
-
-
Chip Two
+ +
-
-
Chip Three
+ +
-
-
Chip Four
+ +
+
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/fixture.scss b/test/screenshot/spec/mdc-chips/fixture.scss index d6b59d5367d..c35772fb5c0 100644 --- a/test/screenshot/spec/mdc-chips/fixture.scss +++ b/test/screenshot/spec/mdc-chips/fixture.scss @@ -35,6 +35,14 @@ $custom-chip-ink-color: $material-color-blue-500; .custom-chip--chip-set-spacing { @include mdc-chip-set-spacing(20px); + + @at-root { + .custom-chip-test-cell { + @include test-cell-size(321, 124); + + padding: 0; + } + } } .custom-chip--shape-radius { @@ -82,11 +90,11 @@ $custom-chip-ink-color: $material-color-blue-500; } .custom-chip--leading-icon-margin { - @include mdc-chip-leading-icon-margin(5px, 5px, 5px, 5px); + @include mdc-chip-leading-icon-margin(5px, 5px); } .custom-chip--trailing-icon-margin { - @include mdc-chip-trailing-icon-margin(5px, 5px, 5px, 5px); + @include mdc-chip-trailing-icon-margin(5px, 5px); } .custom-chip--horizontal-padding { diff --git a/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html b/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html index e5857d8d298..98ea4bf5889 100644 --- a/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html +++ b/test/screenshot/spec/mdc-chips/mixins/chip-set-spacing.html @@ -42,103 +42,103 @@
-
-
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+
+
+ + + +
-
-
-
+
+
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
-
-
+
+
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
-
-
+
+
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html b/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html index 2e1a11d0bf5..807096e0574 100644 --- a/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html +++ b/test/screenshot/spec/mdc-chips/mixins/fill-color-accessible.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/fill-color.html b/test/screenshot/spec/mdc-chips/mixins/fill-color.html index a8062e3e50b..6bfded2e83d 100644 --- a/test/screenshot/spec/mdc-chips/mixins/fill-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/fill-color.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/height.html b/test/screenshot/spec/mdc-chips/mixins/height.html index 35099e882e8..3482a30c5ac 100644 --- a/test/screenshot/spec/mdc-chips/mixins/height.html +++ b/test/screenshot/spec/mdc-chips/mixins/height.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html b/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html index 83fa4f66c05..78c96e28eb7 100644 --- a/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html +++ b/test/screenshot/spec/mdc-chips/mixins/horizontal-padding.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/ink-color.html b/test/screenshot/spec/mdc-chips/mixins/ink-color.html index c9b4e96d00b..9077cde1c0f 100644 --- a/test/screenshot/spec/mdc-chips/mixins/ink-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/ink-color.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html b/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html index 73822cd4712..150c9454ab5 100644 --- a/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/leading-icon-color.html @@ -44,88 +44,88 @@
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html b/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html index a78c8eaf135..ce9b00ab87e 100644 --- a/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html +++ b/test/screenshot/spec/mdc-chips/mixins/leading-icon-margin.html @@ -44,166 +44,166 @@
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html b/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html index 9ab3ca0c3fa..8f3c13f568e 100644 --- a/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html +++ b/test/screenshot/spec/mdc-chips/mixins/leading-icon-size.html @@ -44,88 +44,88 @@
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
+
-
+ + Chip One + +
-
+ + Chip Two + +
-
+ + Chip Three + +
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/outline.html b/test/screenshot/spec/mdc-chips/mixins/outline.html index f97829649cc..3148aedd5c2 100644 --- a/test/screenshot/spec/mdc-chips/mixins/outline.html +++ b/test/screenshot/spec/mdc-chips/mixins/outline.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html b/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html index 322086eb923..8bbbbd3f894 100644 --- a/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/selected-ink-color.html @@ -44,55 +44,55 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
diff --git a/test/screenshot/spec/mdc-chips/mixins/shape-radius.html b/test/screenshot/spec/mdc-chips/mixins/shape-radius.html index 1c03380cfed..fa018867a2f 100644 --- a/test/screenshot/spec/mdc-chips/mixins/shape-radius.html +++ b/test/screenshot/spec/mdc-chips/mixins/shape-radius.html @@ -44,101 +44,101 @@
-
-
Chip One
-
-
-
Chip Two
-
-
-
Chip Three
-
-
-
Chip Four
-
+ + + +
-
+
-
+ Chip One + +
-
+ Chip Two + +
-
+ Chip Three + +
+ Chip Four +
-
-
+
-
Chip One
-
-
-
+ + Chip One + +
-
Chip Two
-
-
-
+ + Chip Two + +
-
Chip Three
-
-
-
+ + Chip Three + +
-
Chip Four
-
+ + Chip Four +
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html index 8d6f219d114..c24faad3822 100644 --- a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html +++ b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-color.html @@ -44,26 +44,26 @@
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html index 69cf7d880ff..e8ca1c1f8f8 100644 --- a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html +++ b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-margin.html @@ -44,26 +44,26 @@
-
+
-
+ +
-
+ +
-
+ +
+
diff --git a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html index 205b282b3ba..180b5690ad9 100644 --- a/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html +++ b/test/screenshot/spec/mdc-chips/mixins/trailing-icon-size.html @@ -44,26 +44,26 @@
-
+
-
+ +
-
+ +
-
+ +
+
From 7400b63ad3e261c35030ae205efdc821c8b8b45b Mon Sep 17 00:00:00 2001 From: Joy Zhong Date: Thu, 2 May 2019 10:17:21 -0400 Subject: [PATCH 06/17] refactor(dialog): Split dialog Foundation#handleInteraction into #handleClick/#handleKeydown. (#4655) BREAKING CHANGE: Dialog `Foundation#handleInteraction` has been split into two methods: `#handleClick` and `#handleKeydown`. --- packages/mdc-dialog/README.md | 8 ++-- packages/mdc-dialog/component.ts | 14 +++--- packages/mdc-dialog/foundation.ts | 35 +++++++++----- test/unit/mdc-dialog/foundation.test.js | 61 +++++++++++++------------ test/unit/mdc-dialog/mdc-dialog.test.js | 8 ++-- 5 files changed, 74 insertions(+), 52 deletions(-) diff --git a/packages/mdc-dialog/README.md b/packages/mdc-dialog/README.md index 392428f801f..fb8d7f74f28 100644 --- a/packages/mdc-dialog/README.md +++ b/packages/mdc-dialog/README.md @@ -400,8 +400,9 @@ Method Signature | Description `setScrimClickAction(action: string)` | Sets the action reflected when the scrim is clicked. Setting to `''` disables closing the dialog via scrim click. `getAutoStackButtons() => boolean` | Returns whether stacked/unstacked action button layout is automatically handled during layout logic. `setAutoStackButtons(autoStack: boolean) => void` | Sets whether stacked/unstacked action button layout is automatically handled during layout logic. -`handleInteraction(event: Event)` | Handles `click` and `keydown` events on or within the dialog's root element -`handleDocumentKeydown(event: Event)` | Handles `keydown` events on or within the document while the dialog is open +`handleClick(event: MouseEvent)` | Handles `click` events on or within the dialog's root element. +`handleKeydown(event: KeyboardEvent)` | Handles `keydown` events on or within the dialog's root element. +`handleDocumentKeydown(event: Event)` | Handles `keydown` events on or within the document while the dialog is open. #### Event Handlers @@ -409,7 +410,8 @@ When wrapping the Dialog foundation, the following events must be bound to the i Event | Target | Foundation Handler | Register | Deregister --- | --- | --- | --- | --- -`click` | `.mdc-dialog` (root) | `handleInteraction` | During initialization | During destruction +`click` | `.mdc-dialog` (root) | `handleClick` | During initialization | During destruction +`keydown` | `.mdc-dialog` (root) | `handleKeydown` | During initialization | During destruction `keydown` | `document` | `handleDocumentKeydown` | On `MDCDialog:opening` | On `MDCDialog:closing` `resize` | `window` | `layout` | On `MDCDialog:opening` | On `MDCDialog:closing` `orientationchange` | `window` | `layout` | On `MDCDialog:opening` | On `MDCDialog:closing` diff --git a/packages/mdc-dialog/component.ts b/packages/mdc-dialog/component.ts index 675cab11164..ff90dab50cc 100644 --- a/packages/mdc-dialog/component.ts +++ b/packages/mdc-dialog/component.ts @@ -77,7 +77,8 @@ export class MDCDialog extends MDCComponent { private focusTrap_!: FocusTrap; // assigned in initialSyncWithDOM() private focusTrapFactory_?: MDCDialogFocusTrapFactory; // assigned in initialize() - private handleInteraction_!: SpecificEventListener<'click' | 'keydown'>; // assigned in initialSyncWithDOM() + private handleClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() + private handleKeydown_!: SpecificEventListener<'keydown'>; // assigned in initialSyncWithDOM() private handleDocumentKeydown_!: SpecificEventListener<'keydown'>; // assigned in initialSyncWithDOM() private handleLayout_!: EventListener; // assigned in initialSyncWithDOM() private handleOpening_!: EventListener; // assigned in initialSyncWithDOM() @@ -107,7 +108,8 @@ export class MDCDialog extends MDCComponent { initialSyncWithDOM() { this.focusTrap_ = util.createFocusTrapInstance(this.container_, this.focusTrapFactory_, this.initialFocusEl_); - this.handleInteraction_ = this.foundation_.handleInteraction.bind(this.foundation_); + this.handleClick_ = this.foundation_.handleClick.bind(this.foundation_); + this.handleKeydown_ = this.foundation_.handleKeydown.bind(this.foundation_); this.handleDocumentKeydown_ = this.foundation_.handleDocumentKeydown.bind(this.foundation_); this.handleLayout_ = this.layout.bind(this); @@ -121,15 +123,15 @@ export class MDCDialog extends MDCComponent { document.removeEventListener('keydown', this.handleDocumentKeydown_); }; - this.listen('click', this.handleInteraction_); - this.listen('keydown', this.handleInteraction_); + this.listen('click', this.handleClick_); + this.listen('keydown', this.handleKeydown_); this.listen(strings.OPENING_EVENT, this.handleOpening_); this.listen(strings.CLOSING_EVENT, this.handleClosing_); } destroy() { - this.unlisten('click', this.handleInteraction_); - this.unlisten('keydown', this.handleInteraction_); + this.unlisten('click', this.handleClick_); + this.unlisten('keydown', this.handleKeydown_); this.unlisten(strings.OPENING_EVENT, this.handleOpening_); this.unlisten(strings.CLOSING_EVENT, this.handleClosing_); this.handleClosing_(); diff --git a/packages/mdc-dialog/foundation.ts b/packages/mdc-dialog/foundation.ts index 76a69f17c7c..3b259b097b5 100644 --- a/packages/mdc-dialog/foundation.ts +++ b/packages/mdc-dialog/foundation.ts @@ -176,26 +176,39 @@ export class MDCDialogFoundation extends MDCFoundation { }); } - handleInteraction(evt: MouseEvent | KeyboardEvent) { - const isClick = evt.type === 'click'; - const isEnter = (evt as KeyboardEvent).key === 'Enter' || (evt as KeyboardEvent).keyCode === 13; - const isSpace = (evt as KeyboardEvent).key === 'Space' || (evt as KeyboardEvent).keyCode === 32; + /** Closes the dialog if scrim or action button click. */ + handleClick(evt: MouseEvent) { const isScrim = this.adapter_.eventTargetMatches(evt.target, strings.SCRIM_SELECTOR); - const isDefault = !this.adapter_.eventTargetMatches(evt.target, strings.SUPPRESS_DEFAULT_PRESS_SELECTOR); - - // Check for scrim click first since it doesn't require querying ancestors - if (isClick && isScrim && this.scrimClickAction_ !== '') { + // Check for scrim click first since it doesn't require querying ancestors. + if (isScrim && this.scrimClickAction_ !== '') { this.close(this.scrimClickAction_); - } else if (isClick || isSpace || isEnter) { + } else { const action = this.adapter_.getActionFromEvent(evt); if (action) { this.close(action); - } else if (isEnter && isDefault) { - this.adapter_.clickDefaultButton(); } } } + handleKeydown(evt: KeyboardEvent) { + const isEnter = evt.key === 'Enter' || evt.keyCode === 13; + if (!isEnter) { + return; + } + const action = this.adapter_.getActionFromEvent(evt); + if (action) { + // Action button callback is handled in `handleClick`, + // since space/enter keydowns on buttons trigger click events. + return; + } + + const isDefault = !this.adapter_.eventTargetMatches( + evt.target, strings.SUPPRESS_DEFAULT_PRESS_SELECTOR); + if (isEnter && isDefault) { + this.adapter_.clickDefaultButton(); + } + } + handleDocumentKeydown(evt: KeyboardEvent) { const isEscape = evt.key === 'Escape' || evt.keyCode === 27; if (isEscape && this.escapeKeyAction_ !== '') { diff --git a/test/unit/mdc-dialog/foundation.test.js b/test/unit/mdc-dialog/foundation.test.js index 56040beae8e..a38c12d638f 100644 --- a/test/unit/mdc-dialog/foundation.test.js +++ b/test/unit/mdc-dialog/foundation.test.js @@ -36,12 +36,6 @@ const ENTER_EVENTS = [ {type: 'keydown', keyCode: 13, target: {}}, ]; -const INTERACTION_EVENTS = [ - {type: 'click', target: {}}, - {type: 'keydown', key: 'Space', target: {}}, - {type: 'keydown', keyCode: 32, target: {}}, -].concat(ENTER_EVENTS); - suite('MDCDialogFoundation'); test('exports cssClasses', () => { @@ -359,82 +353,93 @@ test('#layout removes scrollable class when content is not scrollable', () => { td.verify(mockAdapter.removeClass(cssClasses.SCROLLABLE)); }); -test(`interaction closes dialog when ${strings.ACTION_ATTRIBUTE} attribute is present`, () => { +test(`#handleClick: Click closes dialog when ${strings.ACTION_ATTRIBUTE} attribute is present`, () => { const {foundation, mockAdapter} = setupTest(); const action = 'action'; foundation.close = td.func('close'); - INTERACTION_EVENTS.forEach((event) => { - td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action); - foundation.open(); - foundation.handleInteraction(event); + const event = {target: {}}; + td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action); + foundation.open(); + foundation.handleClick(event); - td.verify(foundation.close(action)); - td.reset(); - }); + td.verify(foundation.close(action)); }); -test('interaction does not close dialog with action for non-activation keys', () => { +test('#handleKeydown: Keydown does not close dialog with action for non-activation keys', () => { const {foundation, mockAdapter} = setupTest(); const action = 'action'; - const event = {type: 'keydown', key: 'Escape', target: {}}; + const event = {type: 'keydown', key: 'Shift', target: {}}; foundation.close = td.func('close'); td.when(mockAdapter.getActionFromEvent(event)).thenReturn(action); foundation.open(); - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(foundation.close(action), {times: 0}); }); -test(`interaction does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => { +test(`#handleClick: Click does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => { const {foundation, mockAdapter} = setupTest(); foundation.close = td.func('close'); - INTERACTION_EVENTS.forEach((event) => { + const event = {target: {}}; + td.when(mockAdapter.getActionFromEvent(event)).thenReturn(''); + foundation.open(); + foundation.handleClick(event); + + td.verify(foundation.close(td.matchers.isA(String)), {times: 0}); +}); + +test(`#handleKeydown: Keydown does nothing when ${strings.ACTION_ATTRIBUTE} attribute is not present`, () => { + const {foundation, mockAdapter} = setupTest(); + foundation.close = td.func('close'); + + ENTER_EVENTS.forEach((event) => { td.when(mockAdapter.getActionFromEvent(event)).thenReturn(''); foundation.open(); - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(foundation.close(td.matchers.isA(String)), {times: 0}); td.reset(); }); }); -test('enter keydown calls adapter.clickDefaultButton', () => { +test('#handleKeydown: Enter keydown calls adapter.clickDefaultButton', () => { const {foundation, mockAdapter} = setupTest(); ENTER_EVENTS.forEach((event) => { - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(mockAdapter.clickDefaultButton()); td.reset(); }); }); -test('enter keydown does not call adapter.clickDefaultButton when it should be suppressed', () => { +test('#handleKeydown: Enter keydown does not call adapter.clickDefaultButton when it should be suppressed', () => { const {foundation, mockAdapter} = setupTest(); ENTER_EVENTS.forEach((event) => { td.when(mockAdapter.eventTargetMatches(event.target, strings.SUPPRESS_DEFAULT_PRESS_SELECTOR)).thenReturn(true); - foundation.handleInteraction(event); + foundation.handleKeydown(event); td.verify(mockAdapter.clickDefaultButton(), {times: 0}); td.reset(); }); }); -test(`click closes dialog when ${strings.SCRIM_SELECTOR} selector matches`, () => { +test(`#handleClick: Click closes dialog when ${strings.SCRIM_SELECTOR} selector matches`, () => { const {foundation, mockAdapter} = setupTest(); const evt = {type: 'click', target: {}}; foundation.close = td.func('close'); td.when(mockAdapter.eventTargetMatches(evt.target, strings.SCRIM_SELECTOR)).thenReturn(true); foundation.open(); - foundation.handleInteraction(evt); + foundation.handleClick(evt); td.verify(foundation.close(foundation.getScrimClickAction())); }); -test(`click does nothing when ${strings.SCRIM_SELECTOR} class is present but scrimClickAction is empty string`, () => { +test(`#handleClick: Click does nothing when ${strings.SCRIM_SELECTOR} class is present but scrimClickAction is + empty string`, () => { const {foundation, mockAdapter} = setupTest(); const evt = {type: 'click', target: {}}; foundation.close = td.func('close'); @@ -442,7 +447,7 @@ test(`click does nothing when ${strings.SCRIM_SELECTOR} class is present but scr foundation.setScrimClickAction(''); foundation.open(); - foundation.handleInteraction(evt); + foundation.handleClick(evt); td.verify(foundation.close(td.matchers.isA(String)), {times: 0}); }); diff --git a/test/unit/mdc-dialog/mdc-dialog.test.js b/test/unit/mdc-dialog/mdc-dialog.test.js index 4dcd9821ac4..47a0287f883 100644 --- a/test/unit/mdc-dialog/mdc-dialog.test.js +++ b/test/unit/mdc-dialog/mdc-dialog.test.js @@ -108,14 +108,14 @@ test('attachTo throws an error when container element is missing', () => { test('#initialSyncWithDOM registers click handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); domEvents.emit(root, 'click'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 1}); + td.verify(mockFoundation.handleClick(td.matchers.isA(Event)), {times: 1}); component.destroy(); }); test('#initialSyncWithDOM registers keydown handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); domEvents.emit(root, 'keydown'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 1}); + td.verify(mockFoundation.handleKeydown(td.matchers.isA(Event)), {times: 1}); component.destroy(); }); @@ -123,14 +123,14 @@ test('#destroy deregisters click handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); component.destroy(); domEvents.emit(root, 'click'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 0}); + td.verify(mockFoundation.handleClick(td.matchers.isA(Event)), {times: 0}); }); test('#destroy deregisters keydown handler on the root element', () => { const {root, component, mockFoundation} = setupTestWithMocks(); component.destroy(); domEvents.emit(root, 'keydown'); - td.verify(mockFoundation.handleInteraction(td.matchers.isA(Event)), {times: 0}); + td.verify(mockFoundation.handleKeydown(td.matchers.isA(Event)), {times: 0}); }); test(`${strings.OPENING_EVENT} registers document keydown handler and ${strings.CLOSING_EVENT} deregisters it`, () => { From 632030898d238fd597254983b4fd3e2a5d6a8a4a Mon Sep 17 00:00:00 2001 From: Joy Zhong Date: Thu, 2 May 2019 11:26:29 -0400 Subject: [PATCH 07/17] docs(dialog): Fix foundation comments. (#4672) --- packages/mdc-dialog/foundation.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/mdc-dialog/foundation.ts b/packages/mdc-dialog/foundation.ts index 3b259b097b5..e6eeffe41f7 100644 --- a/packages/mdc-dialog/foundation.ts +++ b/packages/mdc-dialog/foundation.ts @@ -176,7 +176,7 @@ export class MDCDialogFoundation extends MDCFoundation { }); } - /** Closes the dialog if scrim or action button click. */ + /** Handles click on the dialog root element. */ handleClick(evt: MouseEvent) { const isScrim = this.adapter_.eventTargetMatches(evt.target, strings.SCRIM_SELECTOR); // Check for scrim click first since it doesn't require querying ancestors. @@ -190,6 +190,7 @@ export class MDCDialogFoundation extends MDCFoundation { } } + /** Handles keydown on the dialog root element. */ handleKeydown(evt: KeyboardEvent) { const isEnter = evt.key === 'Enter' || evt.keyCode === 13; if (!isEnter) { @@ -209,6 +210,7 @@ export class MDCDialogFoundation extends MDCFoundation { } } + /** Handles keydown on the document. */ handleDocumentKeydown(evt: KeyboardEvent) { const isEscape = evt.key === 'Escape' || evt.keyCode === 27; if (isEscape && this.escapeKeyAction_ !== '') { From 6de41158ab5bed254d18c5e7365017b95ae201a8 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Mon, 6 May 2019 10:11:45 -0700 Subject: [PATCH 08/17] fix(chips): update goldens (#4679) --- test/screenshot/golden.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/screenshot/golden.json b/test/screenshot/golden.json index 515b8964a23..714e1ed58c6 100644 --- a/test/screenshot/golden.json +++ b/test/screenshot/golden.json @@ -240,11 +240,11 @@ } }, "spec/mdc-chips/mixins/chip-set-spacing.html": { - "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/05/03/18_19_22_511/spec/mdc-chips/mixins/chip-set-spacing.html?utm_source=golden_json", + "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/05/03/21_28_57_234/spec/mdc-chips/mixins/chip-set-spacing.html?utm_source=golden_json", "screenshots": { - "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/05/03/18_19_22_511/spec/mdc-chips/mixins/chip-set-spacing.html.windows_chrome_74.png", - "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/rlfriedman/2018/09/13/20_54_11_470/spec/mdc-chips/mixins/chip-set-spacing.html.windows_firefox_62.png", - "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/rlfriedman/2018/09/13/20_54_11_470/spec/mdc-chips/mixins/chip-set-spacing.html.windows_ie_11.png" + "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/05/03/21_28_57_234/spec/mdc-chips/mixins/chip-set-spacing.html.windows_chrome_74.png", + "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/05/03/21_28_57_234/spec/mdc-chips/mixins/chip-set-spacing.html.windows_firefox_65.png", + "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/05/03/21_28_57_234/spec/mdc-chips/mixins/chip-set-spacing.html.windows_ie_11.png" } }, "spec/mdc-chips/mixins/fill-color-accessible.html": { From a031927128c83ab4c3a53b9f9fff2f182b586074 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Mon, 6 May 2019 11:33:33 -0700 Subject: [PATCH 09/17] feat(menu): add setSelectedIndex to set selected item in menu selection group (#4620) BREAKING CHANGE: Replaced adapter methods getParentElement, getSelectedElementIndex with getSelectedSiblingOfItemAtIndex, isSelectableItemAtIndex. --- packages/mdc-menu/README.md | 6 +- packages/mdc-menu/adapter.ts | 23 +-- packages/mdc-menu/component.ts | 20 ++- packages/mdc-menu/foundation.ts | 57 +++--- packages/mdc-menu/package.json | 1 + test/screenshot/golden.json | 8 + .../multiple-menu-selection-group.html | 165 ++++++++++++++++++ test/screenshot/spec/mdc-menu/fixture.js | 5 + test/unit/mdc-menu/mdc-menu.test.js | 96 ++++++---- test/unit/mdc-menu/menu.foundation.test.js | 140 ++++++--------- 10 files changed, 350 insertions(+), 171 deletions(-) create mode 100644 test/screenshot/spec/mdc-menu/classes/multiple-menu-selection-group.html diff --git a/packages/mdc-menu/README.md b/packages/mdc-menu/README.md index ce58b342775..40fc57dcd81 100644 --- a/packages/mdc-menu/README.md +++ b/packages/mdc-menu/README.md @@ -226,6 +226,7 @@ Method Signature | Description `setAnchorMargin(Partial) => void` | Proxies to the menu surface's `setAnchorMargin(Partial)` method. `setAbsolutePosition(x: number, y: number) => void` | Proxies to the menu surface's `setAbsolutePosition(x: number, y: number)` method. `setFixedPosition(isFixed: boolean) => void` | Proxies to the menu surface's `setFixedPosition(isFixed: boolean)` method. +`setSelectedIndex(index: number) => void | Sets the list item to the selected state at the specified index. `hoistMenuToBody() => void` | Proxies to the menu surface's `hoistMenuToBody()` method. `setIsHoisted(isHoisted: boolean) => void` | Proxies to the menu surface's `setIsHoisted(isHoisted: boolean)` method. `setAnchorElement(element: Element) => void` | Proxies to the menu surface's `setAnchorElement(element)` method. @@ -250,12 +251,12 @@ Method Signature | Description `elementContainsClass(element: Element, className: string) => boolean` | Returns true if the `element` contains the `className` class. `closeSurface() => void` | Closes the menu surface. `getElementIndex(element: Element) => number` | Returns the `index` value of the `element`. -`getParentElement(element: Element) => Element \| null` | Returns the `.parentElement` element of the `element` provided. -`getSelectedElementIndex(element: Element) => number` | Returns the `index` value of the element within the selection group provided, `element` that contains the `mdc-menu-item--selected` class. `notifySelected(index: number) => void` | Emits a `MDCMenu:selected` event for the element at the `index` specified. `getMenuItemCount() => number` | Returns the menu item count. `focusItemAtIndex(index: number)` | Focuses the menu item at given index. `focusListRoot() => void` | Focuses the list root element. +`getSelectedSiblingOfItemAtIndex(index: number) => number` | Returns selected list item index within the same selection group which is a sibling of item at given `index`. +`isSelectableItemAtIndex(index: number) => boolean` | Returns true if menu item at specified index is contained within an `.mdc-menu__selection-group` element. ### `MDCMenuFoundation` @@ -265,6 +266,7 @@ Method Signature | Description `handleItemAction(listItem: Element) => void` | Event handler for list's action event. `handleMenuSurfaceOpened() => void` | Event handler for menu surface's opened event. `setDefaultFocusState(focusState: DefaultFocusState) => void` | Sets default focus state where the menu should focus every time when menu is opened. Focuses the list root (`DefaultFocusState.LIST_ROOT`) element by default. +`setSelectedIndex(index: number) => void` | Selects the list item at given `index`. ### Events diff --git a/packages/mdc-menu/adapter.ts b/packages/mdc-menu/adapter.ts index 5690de25a71..324a8154fa7 100644 --- a/packages/mdc-menu/adapter.ts +++ b/packages/mdc-menu/adapter.ts @@ -65,16 +65,6 @@ export interface MDCMenuAdapter { */ getElementIndex(element: Element): number; - /** - * @return The parentElement of the provided element. - */ - getParentElement(element: Element): Element | null; - - /** - * @return The element within the selectionGroup containing the selected element class. - */ - getSelectedElementIndex(selectionGroup: Element): number; - /** * Emit an event when a menu item is selected. */ @@ -91,4 +81,17 @@ export interface MDCMenuAdapter { /** Focuses the list root element. */ focusListRoot(): void; + + /** + * @return Returns selected list item index within the same selection group which is + * a sibling of item at given `index`. + * @param index Index of the menu item with possible selected sibling. + */ + getSelectedSiblingOfItemAtIndex(index: number): number; + + /** + * @return Returns true if item at specified index is contained within an `.mdc-menu__selection-group` element. + * @param index Index of the selectable menu item. + */ + isSelectableItemAtIndex(index: number): boolean; } diff --git a/packages/mdc-menu/component.ts b/packages/mdc-menu/component.ts index aa900acf4a2..b1260874de3 100644 --- a/packages/mdc-menu/component.ts +++ b/packages/mdc-menu/component.ts @@ -23,6 +23,7 @@ import {MDCComponent} from '@material/base/component'; import {CustomEventListener, SpecificEventListener} from '@material/base/types'; +import {closest} from '@material/dom/ponyfill'; import {MDCList, MDCListFactory} from '@material/list/component'; import {MDCListFoundation} from '@material/list/foundation'; import {MDCListActionEvent} from '@material/list/types'; @@ -143,6 +144,14 @@ export class MDCMenu extends MDCComponent { this.menuSurface_.setAnchorMargin(margin); } + /** + * Sets the list item as the selected row at the specified index. + * @param index Index of list item within menu. + */ + setSelectedIndex(index: number) { + this.foundation_.setSelectedIndex(index); + } + /** * @return The item within the menu at the index specified. */ @@ -203,11 +212,6 @@ export class MDCMenu extends MDCComponent { elementContainsClass: (element, className) => element.classList.contains(className), closeSurface: () => this.open = false, getElementIndex: (element) => this.items.indexOf(element), - getParentElement: (element) => element.parentElement, - getSelectedElementIndex: (selectionGroup) => { - const selectedListItem = selectionGroup.querySelector(`.${cssClasses.MENU_SELECTED_LIST_ITEM}`); - return selectedListItem ? this.items.indexOf(selectedListItem) : -1; - }, notifySelected: (evtData) => this.emit(strings.SELECTED_EVENT, { index: evtData.index, item: this.items[evtData.index], @@ -215,6 +219,12 @@ export class MDCMenu extends MDCComponent { getMenuItemCount: () => this.items.length, focusItemAtIndex: (index) => (this.items[index] as HTMLElement).focus(), focusListRoot: () => (this.root_.querySelector(strings.LIST_SELECTOR) as HTMLElement).focus(), + isSelectableItemAtIndex: (index) => !!closest(this.items[index], `.${cssClasses.MENU_SELECTION_GROUP}`), + getSelectedSiblingOfItemAtIndex: (index) => { + const selectionGroupEl = closest(this.items[index], `.${cssClasses.MENU_SELECTION_GROUP}`) as HTMLElement; + const selectedItemEl = selectionGroupEl.querySelector(`.${cssClasses.MENU_SELECTED_LIST_ITEM}`); + return selectedItemEl ? this.items.indexOf(selectedItemEl) : -1; + }, }; // tslint:enable:object-literal-sort-keys return new MDCMenuFoundation(adapter); diff --git a/packages/mdc-menu/foundation.ts b/packages/mdc-menu/foundation.ts index 40cc66de800..a4d2cc81b33 100644 --- a/packages/mdc-menu/foundation.ts +++ b/packages/mdc-menu/foundation.ts @@ -22,7 +22,6 @@ */ import {MDCFoundation} from '@material/base/foundation'; -import {MDCListFoundation} from '@material/list/foundation'; import {MDCMenuSurfaceFoundation} from '@material/menu-surface/foundation'; import {MDCMenuAdapter} from './adapter'; import {cssClasses, DefaultFocusState, numbers, strings} from './constants'; @@ -56,12 +55,12 @@ export class MDCMenuFoundation extends MDCFoundation { elementContainsClass: () => false, closeSurface: () => undefined, getElementIndex: () => -1, - getParentElement: () => null, - getSelectedElementIndex: () => -1, notifySelected: () => undefined, getMenuItemCount: () => 0, focusItemAtIndex: () => undefined, focusListRoot: () => undefined, + getSelectedSiblingOfItemAtIndex: () => -1, + isSelectableItemAtIndex: () => false, }; // tslint:enable:object-literal-sort-keys } @@ -98,9 +97,8 @@ export class MDCMenuFoundation extends MDCFoundation { // Wait for the menu to close before adding/removing classes that affect styles. this.closeAnimationEndTimerId_ = setTimeout(() => { - const selectionGroup = this.getSelectionGroup_(listItem); - if (selectionGroup) { - this.handleSelectionGroup_(selectionGroup, index); + if (this.adapter_.isSelectableItemAtIndex(index)) { + this.setSelectedIndex(index); } }, MDCMenuSurfaceFoundation.numbers.TRANSITION_CLOSE_DURATION); } @@ -132,41 +130,32 @@ export class MDCMenuFoundation extends MDCFoundation { } /** - * Handles toggling the selected classes in a selection group when a selection is made. + * Selects the list item at `index` within the menu. + * @param index Index of list item within the menu. */ - private handleSelectionGroup_(selectionGroup: Element, index: number) { - // De-select the previous selection in this group. - const selectedIndex = this.adapter_.getSelectedElementIndex(selectionGroup); - if (selectedIndex >= 0) { - this.adapter_.removeAttributeFromElementAtIndex(selectedIndex, strings.ARIA_SELECTED_ATTR); - this.adapter_.removeClassFromElementAtIndex(selectedIndex, cssClasses.MENU_SELECTED_LIST_ITEM); + setSelectedIndex(index: number) { + this.validatedIndex_(index); + + if (!this.adapter_.isSelectableItemAtIndex(index)) { + throw new Error('MDCMenuFoundation: No selection group at specified index.'); } - // Select the new list item in this group. - this.adapter_.addClassToElementAtIndex(index, cssClasses.MENU_SELECTED_LIST_ITEM); - this.adapter_.addAttributeToElementAtIndex(index, strings.ARIA_SELECTED_ATTR, 'true'); - } - /** - * Returns the parent selection group of an element if one exists. - */ - private getSelectionGroup_(listItem: Element): Element | null { - let parent = this.adapter_.getParentElement(listItem); - if (!parent) { - return null; + const prevSelectedIndex = this.adapter_.getSelectedSiblingOfItemAtIndex(index); + if (prevSelectedIndex >= 0) { + this.adapter_.removeAttributeFromElementAtIndex(prevSelectedIndex, strings.ARIA_SELECTED_ATTR); + this.adapter_.removeClassFromElementAtIndex(prevSelectedIndex, cssClasses.MENU_SELECTED_LIST_ITEM); } - let isGroup = this.adapter_.elementContainsClass(parent, cssClasses.MENU_SELECTION_GROUP); + this.adapter_.addClassToElementAtIndex(index, cssClasses.MENU_SELECTED_LIST_ITEM); + this.adapter_.addAttributeToElementAtIndex(index, strings.ARIA_SELECTED_ATTR, 'true'); + } - // Iterate through ancestors until we find the group or get to the list. - while (!isGroup && parent && !this.adapter_.elementContainsClass(parent, MDCListFoundation.cssClasses.ROOT)) { - parent = this.adapter_.getParentElement(parent); - isGroup = parent ? this.adapter_.elementContainsClass(parent, cssClasses.MENU_SELECTION_GROUP) : false; - } + private validatedIndex_(index: number): void { + const menuSize = this.adapter_.getMenuItemCount(); + const isIndexInRange = index >= 0 && index < menuSize; - if (isGroup) { - return parent; - } else { - return null; + if (!isIndexInRange) { + throw new Error('MDCMenuFoundation: No list item at specified index.'); } } } diff --git a/packages/mdc-menu/package.json b/packages/mdc-menu/package.json index 034b118c888..3159f92fd4e 100644 --- a/packages/mdc-menu/package.json +++ b/packages/mdc-menu/package.json @@ -18,6 +18,7 @@ }, "dependencies": { "@material/base": "^1.0.0", + "@material/dom": "^1.1.0", "@material/feature-targeting": "^0.44.1", "@material/list": "^2.0.0", "@material/menu-surface": "^1.1.1", diff --git a/test/screenshot/golden.json b/test/screenshot/golden.json index 714e1ed58c6..93579060cd9 100644 --- a/test/screenshot/golden.json +++ b/test/screenshot/golden.json @@ -807,6 +807,14 @@ "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/03/15/18_35_08_782/spec/mdc-menu/classes/menu-selection-group.html.windows_ie_11.png" } }, + "spec/mdc-menu/classes/multiple-menu-selection-group.html": { + "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/23/22_44_56_133/spec/mdc-menu/classes/multiple-menu-selection-group.html?utm_source=golden_json", + "screenshots": { + "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/23/22_44_56_133/spec/mdc-menu/classes/multiple-menu-selection-group.html.windows_chrome_73.png", + "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/23/22_44_56_133/spec/mdc-menu/classes/multiple-menu-selection-group.html.windows_firefox_65.png", + "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/23/22_44_56_133/spec/mdc-menu/classes/multiple-menu-selection-group.html.windows_ie_11.png" + } + }, "spec/mdc-menu/issues/4025.html": { "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/01/18/07_50_32_667/spec/mdc-menu/issues/4025.html?utm_source=golden_json", "screenshots": { diff --git a/test/screenshot/spec/mdc-menu/classes/multiple-menu-selection-group.html b/test/screenshot/spec/mdc-menu/classes/multiple-menu-selection-group.html new file mode 100644 index 00000000000..77856b9d715 --- /dev/null +++ b/test/screenshot/spec/mdc-menu/classes/multiple-menu-selection-group.html @@ -0,0 +1,165 @@ + + + + + + Multiple Menu Selection Group - MDC Web Screenshot Test + + + + + + + + + + + + + + +
+
+
+
+ +
+ +
+
+
+
+
+ + + + + + + + + + diff --git a/test/screenshot/spec/mdc-menu/fixture.js b/test/screenshot/spec/mdc-menu/fixture.js index 850b29bfd5a..1c4aac22808 100644 --- a/test/screenshot/spec/mdc-menu/fixture.js +++ b/test/screenshot/spec/mdc-menu/fixture.js @@ -26,6 +26,7 @@ import {DefaultFocusState} from '../../../../packages/mdc-menu/constants'; window.mdc.testFixture.fontsLoaded.then(() => { const buttonEl = document.querySelector('.test-menu-button'); const menuEl = document.querySelector('.mdc-menu'); + const multipleSelectionGroupMenuEl = document.getElementById('test-multiple-selection-group-menu'); const menu = mdc.menu.MDCMenu.attachTo(menuEl); menu.setAnchorCorner(mdc.menu.Corner.BOTTOM_LEFT); menu.open = true; @@ -51,6 +52,10 @@ window.mdc.testFixture.fontsLoaded.then(() => { menu.open = !menu.open; } }); + if (multipleSelectionGroupMenuEl) { + menu.setSelectedIndex(3); + menu.setSelectedIndex(5); + } window.mdc.testFixture.notifyDomReady(); }); diff --git a/test/unit/mdc-menu/mdc-menu.test.js b/test/unit/mdc-menu/mdc-menu.test.js index 8cbba21ffde..31ae9abb2cb 100644 --- a/test/unit/mdc-menu/mdc-menu.test.js +++ b/test/unit/mdc-menu/mdc-menu.test.js @@ -39,10 +39,37 @@ function getFixture(open) {
  • + +
    • + +
    + `; +} + +function getFixtureWithMultipleSelectionGroups(open) { + return bel` +
    +
    `; @@ -104,8 +131,8 @@ function setupTestWithFakes(open = false) { * @param {boolean=} open * @return {{component: !MDCMenu, root: !HTMLElement}} */ -function setupTest(open = false) { - const root = getFixture(open); +function setupTest(open = false, fixture = getFixture) { + const root = fixture(open); const component = new MDCMenu(root); return {root, component}; @@ -115,8 +142,8 @@ function setupTest(open = false) { * @param {!Object=} options * @return {{component: !MDCMenu, root: !HTMLElement, mockFoundation: !MDCMenuFoundation}} */ -function setupTestWithMock(options = {open: true}) { - const root = getFixture(options.open); +function setupTestWithMock(options = {open: true, fixture: getFixture}) { + const root = options.fixture(options.open); const MockFoundationCtor = td.constructor(MDCMenuFoundation); const mockFoundation = new MockFoundationCtor(); @@ -209,6 +236,12 @@ test('setAnchorMargin', () => { td.verify(menuSurface.setAnchorMargin({top: 0, right: 0, bottom: 0, left: 0})); }); +test('setSelectedIndex calls foundation method setSelectedIndex with given index.', () => { + const {component, mockFoundation} = setupTestWithMock({fixture: getFixtureWithMultipleSelectionGroups}); + component.setSelectedIndex(1); + td.verify(mockFoundation.setSelectedIndex(1)); +}); + test('setQuickOpen', () => { const {component, menuSurface} = setupTestWithFakes(); component.quickOpen = true; @@ -397,29 +430,6 @@ test('adapter#getElementIndex returns -1 if the element does not exist in the li assert.equal(indexValue, -1); }); -test('adapter#getParentElement returns the parent element of an element', () => { - const {root, component} = setupTest(); - const firstItem = root.querySelector('.mdc-list-item'); - const parentElement = component.getDefaultFoundation().adapter_.getParentElement(firstItem); - assert.equal(firstItem.parentElement, parentElement); -}); - -test('adapter#getSelectedElementIndex returns the index of the "selected" element in a group', () => { - const {root, component} = setupTest(); - const selectionGroup = root.querySelector('.mdc-menu__selection-group'); - const index = component.getDefaultFoundation().adapter_.getSelectedElementIndex(selectionGroup); - assert.equal(root.querySelector('.mdc-menu-item--selected'), component.items[index]); -}); - -test('adapter#getSelectedElementIndex returns -1 if the "selected" element is not in a group', () => { - const {root, component} = setupTest(); - const selectionGroup = root.querySelector('.mdc-menu__selection-group'); - const element = root.querySelector('.mdc-menu-item--selected'); - element.classList.remove('mdc-menu-item--selected'); - const index = component.getDefaultFoundation().adapter_.getSelectedElementIndex(selectionGroup); - assert.equal(index, -1); -}); - test('adapter#notifySelected emits an event for a selected element', () => { const {root, component} = setupTest(); const handler = td.func('eventHandler'); @@ -452,3 +462,27 @@ test('adapter#focusListRoot focuses the list root element', () => { document.body.removeChild(root); }); + +test('adapter#isSelectableItemAtIndex returns true if the menu item is within the' + +'.mdc-menu__selection-group element', () => { + const {component} = setupTest(); + + const isSelectableItemAtIndex = component.getDefaultFoundation().adapter_.isSelectableItemAtIndex(3); + assert.isTrue(isSelectableItemAtIndex); +}); + +test('adapter#isSelectableItemAtIndex returns false if the menu item is not within the' + +'.mdc-menu__selection-group element', () => { + const {component} = setupTest(); + + const isSelectableItemAtIndex = component.getDefaultFoundation().adapter_.isSelectableItemAtIndex(1); + assert.isFalse(isSelectableItemAtIndex); +}); + +test('adapter#getSelectedSiblingOfItemAtIndex returns the index of the selected item within the same' + +'selection group', () => { + const {component} = setupTest(); + + const siblingIndex = component.getDefaultFoundation().adapter_.getSelectedSiblingOfItemAtIndex(2); + assert.equal(siblingIndex, 3); +}); diff --git a/test/unit/mdc-menu/menu.foundation.test.js b/test/unit/mdc-menu/menu.foundation.test.js index b62625efdbe..60306fffe03 100644 --- a/test/unit/mdc-menu/menu.foundation.test.js +++ b/test/unit/mdc-menu/menu.foundation.test.js @@ -45,8 +45,9 @@ suite('MDCMenuFoundation'); test('defaultAdapter returns a complete adapter implementation', () => { verifyDefaultAdapter(MDCMenuFoundation, [ 'addClassToElementAtIndex', 'removeClassFromElementAtIndex', 'addAttributeToElementAtIndex', - 'removeAttributeFromElementAtIndex', 'elementContainsClass', 'closeSurface', 'getElementIndex', 'getParentElement', - 'getSelectedElementIndex', 'notifySelected', 'getMenuItemCount', 'focusItemAtIndex', 'focusListRoot', + 'removeAttributeFromElementAtIndex', 'elementContainsClass', 'closeSurface', 'getElementIndex', + 'getSelectedSiblingOfItemAtIndex', 'isSelectableItemAtIndex', 'notifySelected', + 'getMenuItemCount', 'focusItemAtIndex', 'focusListRoot', ]); }); @@ -139,12 +140,10 @@ test('handleItemAction item action event inside of a selection group ' + 'with additional markup does not cause loop', () => { // This test will timeout of there is an endless loop in the selection group logic. const {foundation, mockAdapter, clock} = setupTest(); - const parentElement = {}; const itemEl = document.createElement('li'); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); td.when(mockAdapter.elementContainsClass(td.matchers.anything(), listClasses.ROOT)).thenReturn(false, true); td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(td.matchers.anything())).thenReturn(parentElement, null); td.when(mockAdapter.elementContainsClass(td.matchers.anything(), cssClasses.MENU_SELECTION_GROUP)).thenReturn(false); foundation.handleItemAction(itemEl); @@ -158,14 +157,15 @@ test('handleItemAction item action event inside of a selection group with anothe const itemEl = document.createElement('li'); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(0); + td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(true); + td.when(mockAdapter.getSelectedSiblingOfItemAtIndex(0)).thenReturn(1); + td.when(mockAdapter.getMenuItemCount()).thenReturn(5); foundation.handleItemAction(itemEl); clock.tick(numbers.TRANSITION_CLOSE_DURATION); - td.verify(mockAdapter.removeClassFromElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); + td.verify(mockAdapter.removeClassFromElementAtIndex(1, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); td.verify(mockAdapter.addClassToElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); }); @@ -174,9 +174,10 @@ test('handleItemAction item action event inside of a selection group with no ele const itemEl = document.createElement('li'); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(-1); + td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(true); + td.when(mockAdapter.getSelectedSiblingOfItemAtIndex(0)).thenReturn(-1); + td.when(mockAdapter.getMenuItemCount()).thenReturn(5); foundation.handleItemAction(itemEl); clock.tick(numbers.TRANSITION_CLOSE_DURATION); @@ -192,9 +193,9 @@ test('handleItemAction item action event inside of a child element of a list ite const itemEl = document.createElement('li'); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(false, true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(-1); + td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(true); + td.when(mockAdapter.getMenuItemCount()).thenReturn(5); foundation.handleItemAction(itemEl); clock.tick(numbers.TRANSITION_CLOSE_DURATION); @@ -210,10 +211,10 @@ test('handleItemAction item action event inside of a child element of a selectio const itemEl = document.createElement('li'); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(false); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.ROOT)).thenReturn(false, true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(-1); + td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(false); + td.when(mockAdapter.getMenuItemCount()).thenReturn(5); foundation.handleItemAction(itemEl); clock.tick(numbers.TRANSITION_CLOSE_DURATION); @@ -266,96 +267,57 @@ test('handleMenuSurfaceOpened does not focus anything when DefaultFocusState is td.verify(mockAdapter.focusListRoot(), {times: 0}); }); -// Item Action - -test('Item action event causes the menu to close', () => { +test('setSelectedIndex calls addClass and addAttribute only', () => { const {foundation, mockAdapter} = setupTest(); - const itemEl = document.createElement('li'); - td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); - td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - - foundation.handleItemAction(itemEl); - - td.verify(mockAdapter.closeSurface(), {times: 1}); + const listItemEl = document.createElement('div'); + td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(true); + td.when(mockAdapter.elementContainsClass(listItemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true); + td.when(mockAdapter.getSelectedSiblingOfItemAtIndex(0)).thenReturn(-1); + td.when(mockAdapter.getMenuItemCount()).thenReturn(2); + + foundation.setSelectedIndex(0); + td.verify(mockAdapter.removeClassFromElementAtIndex( + td.matchers.isA(Number), cssClasses.MENU_SELECTED_LIST_ITEM), {times: 0}); + td.verify(mockAdapter.removeAttributeFromElementAtIndex(td.matchers.isA(Number), + strings.ARIA_SELECTED_ATTR), {times: 0}); + td.verify(mockAdapter.addClassToElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); + td.verify(mockAdapter.addAttributeToElementAtIndex(0, strings.ARIA_SELECTED_ATTR, 'true'), {times: 1}); }); -test('item action event causes the menu to emit the selected item event', () => { +test('setSelectedIndex remove class and attribute, and adds class and attribute to newly selected item', () => { const {foundation, mockAdapter} = setupTest(); - const itemEl = document.createElement('li'); - td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); - td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - - foundation.handleItemAction(itemEl); - - td.verify(mockAdapter.notifySelected({index: 0}), {times: 1}); -}); - -test('item action event inside of a selection group with another element selected', () => { - const {foundation, mockAdapter, clock} = setupTest(); - const itemEl = document.createElement('li'); - td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); - td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); - td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(0); - - foundation.handleItemAction(itemEl); - clock.tick(numbers.TRANSITION_CLOSE_DURATION); - - td.verify(mockAdapter.removeClassFromElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); + const listItemEl = document.createElement('div'); + td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(true); + td.when(mockAdapter.elementContainsClass(listItemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true); + td.when(mockAdapter.getMenuItemCount()).thenReturn(2); + td.when(mockAdapter.getSelectedSiblingOfItemAtIndex(0)).thenReturn(1); + + foundation.setSelectedIndex(0); + td.verify(mockAdapter.removeClassFromElementAtIndex(1, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); + td.verify( + mockAdapter.removeAttributeFromElementAtIndex(1, strings.ARIA_SELECTED_ATTR), {times: 1}); td.verify(mockAdapter.addClassToElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); + td.verify(mockAdapter.addAttributeToElementAtIndex(0, strings.ARIA_SELECTED_ATTR, 'true'), {times: 1}); }); -test('item action event inside of a selection group with no element selected', () => { - const {foundation, mockAdapter, clock} = setupTest(); - const itemEl = document.createElement('li'); - td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); - td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); - td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(-1); - - foundation.handleItemAction(itemEl); - clock.tick(numbers.TRANSITION_CLOSE_DURATION); - - td.verify(mockAdapter.removeClassFromElementAtIndex(td.matchers.isA(Number), cssClasses.MENU_SELECTED_LIST_ITEM), - {times: 0}); - td.verify(mockAdapter.addClassToElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); +test('setSelectedIndex throws error if index is not in range', () => { + const {foundation} = setupTest(); + try { + foundation.setSelectedIndex(5); + } catch (e) { + assert.equal(e.message, 'MDCMenuFoundation: No list item at specified index.'); + } }); -test('item action event inside of a child element of a list item in a selection group with no element selected', () => { - const {foundation, mockAdapter, clock} = setupTest(); - const itemEl = document.createElement('li'); - td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); - td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); - td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(false, true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(-1); - - foundation.handleItemAction(itemEl); - clock.tick(numbers.TRANSITION_CLOSE_DURATION); - - td.verify(mockAdapter.removeClassFromElementAtIndex(td.matchers.isA(Number), cssClasses.MENU_SELECTED_LIST_ITEM), - {times: 0}); - td.verify(mockAdapter.addClassToElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1}); -}); +// Item Action -test('item action event inside of a child element of a selection group (but not a list item) with no element ' + - 'selected', () => { - const {foundation, mockAdapter, clock} = setupTest(); +test('Item action event causes the menu to close', () => { + const {foundation, mockAdapter} = setupTest(); const itemEl = document.createElement('li'); td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true); td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0); - td.when(mockAdapter.getParentElement(itemEl)).thenReturn(itemEl); - td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(false); - td.when(mockAdapter.elementContainsClass(itemEl, listClasses.ROOT)).thenReturn(false, true); - td.when(mockAdapter.getSelectedElementIndex(itemEl)).thenReturn(-1); foundation.handleItemAction(itemEl); - clock.tick(numbers.TRANSITION_CLOSE_DURATION); - td.verify(mockAdapter.removeClassFromElementAtIndex(td.matchers.isA(Number), cssClasses.MENU_SELECTED_LIST_ITEM), - {times: 0}); - td.verify(mockAdapter.addClassToElementAtIndex(td.matchers.isA(Number), cssClasses.MENU_SELECTED_LIST_ITEM), - {times: 0}); + td.verify(mockAdapter.closeSurface(), {times: 1}); }); From cc44b25609246d611121af6177f26d6342a210b3 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Fri, 10 May 2019 11:31:07 -0700 Subject: [PATCH 10/17] fix: update register/deregsiter events api for top app bar --- packages/mdc-top-app-bar/README.md | 11 +- packages/mdc-top-app-bar/adapter.ts | 20 --- packages/mdc-top-app-bar/component.ts | 52 ++++-- packages/mdc-top-app-bar/fixed/foundation.ts | 2 +- packages/mdc-top-app-bar/foundation.ts | 49 +----- packages/mdc-top-app-bar/short/foundation.ts | 9 +- .../mdc-top-app-bar/standard/foundation.ts | 4 +- .../mdc-top-app-bar/fixed.foundation.test.js | 71 ++------ test/unit/mdc-top-app-bar/foundation.test.js | 34 +--- .../mdc-top-app-bar/mdc-top-app-bar.test.js | 152 ++++++------------ .../mdc-top-app-bar/short.foundation.test.js | 67 +++----- .../standard.foundation.test.js | 93 +++-------- 12 files changed, 156 insertions(+), 408 deletions(-) diff --git a/packages/mdc-top-app-bar/README.md b/packages/mdc-top-app-bar/README.md index dcd19131210..533cab6746b 100644 --- a/packages/mdc-top-app-bar/README.md +++ b/packages/mdc-top-app-bar/README.md @@ -219,12 +219,6 @@ Method Signature | Description `getViewportScrollY() => number` | Gets the number of pixels that the content of body is scrolled from the top of the page. `getTotalActionItems() => number` | Gets the number of action items in the top app bar. `notifyNavigationIconClicked() => void` | Emits a custom event `MDCTopAppBar:nav` when the navigation icon is clicked. -`registerNavigationIconInteractionHandler(evtType: string, handler: EventListener) => void` | Registers an event listener on the native navigation icon element for a given event. -`deregisterNavigationIconInteractionHandler(evtType: string, handler: EventListener) => void` | Deregisters an event listener on the native navigation icon element for a given event. -`registerScrollHandler(handler: EventListener) => void` | Registers a handler to be called when user scrolls. Our default implementation adds the handler as a listener to the window's `scroll` event. -`deregisterScrollHandler(handler: EventListener) => void` | Unregisters a handler to be called when user scrolls. Our default implementation removes the handler as a listener to the window's `scroll` event. -`registerResizeHandler(handler: EventListener) => void` | Registers a handler to be called when the surface (or its viewport) resizes. Our default implementation adds the handler as a listener to the window's `resize` event. -`deregisterResizeHandler(handler: EventListener) => void` | Unregisters a handler to be called when the surface (or its viewport) resizes. Our default implementation removes the handler as a listener to the window's `resize` event. ### Foundations @@ -234,8 +228,9 @@ All foundations provide the following methods: Method Signature | Description --- | --- -`initScrollHandler(handler: EventListener) => void` | Registers a scroll handler on a specific target element. -`destroyScrollHandler(handler: EventListener) => void` | Deregisters the current scroll handler set by the foundation. +`handleScroll() => void` | Handles `scroll` event on specified scrollTarget (default to `window`). +`handleResize() => void` | Handles `resize` event on window. +`handleNavigationClick() => void` | Handles `click` event on navigation icon. #### `MDCShortTopAppBarFoundation` diff --git a/packages/mdc-top-app-bar/adapter.ts b/packages/mdc-top-app-bar/adapter.ts index ad9203d1a7e..dcbc9ba37d6 100644 --- a/packages/mdc-top-app-bar/adapter.ts +++ b/packages/mdc-top-app-bar/adapter.ts @@ -21,8 +21,6 @@ * THE SOFTWARE. */ -import {EventType, SpecificEventListener} from '@material/base/types'; - /** * Defines the shape of the adapter expected by the foundation. * Implement this adapter for your framework of choice to delegate updates to @@ -64,22 +62,4 @@ export interface MDCTopAppBarAdapter { * Emits an event when the navigation icon is clicked. */ notifyNavigationIconClicked(): void; - - /** - * Registers an event handler on the navigation icon element for a given event. - */ - registerNavigationIconInteractionHandler(type: K, handler: SpecificEventListener): void; - - /** - * Deregisters an event handler on the navigation icon element for a given event. - */ - deregisterNavigationIconInteractionHandler(type: K, handler: SpecificEventListener): void; - - registerScrollHandler(handler: SpecificEventListener<'scroll'>): void; - - deregisterScrollHandler(handler: SpecificEventListener<'scroll'>): void; - - registerResizeHandler(handler: SpecificEventListener<'resize'>): void; - - deregisterResizeHandler(handler: SpecificEventListener<'resize'>): void; } diff --git a/packages/mdc-top-app-bar/component.ts b/packages/mdc-top-app-bar/component.ts index 21fa033c889..17c9bdf9048 100644 --- a/packages/mdc-top-app-bar/component.ts +++ b/packages/mdc-top-app-bar/component.ts @@ -22,6 +22,7 @@ */ import {MDCComponent} from '@material/base/component'; +import {SpecificEventListener} from '@material/base/types'; import {MDCRipple, MDCRippleFactory} from '@material/ripple/component'; import {MDCTopAppBarAdapter} from './adapter'; import {cssClasses, strings} from './constants'; @@ -35,6 +36,9 @@ export class MDCTopAppBar extends MDCComponent { return new MDCTopAppBar(root); } + private handleNavigationIconClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() + private handleResize_?: SpecificEventListener<'resize'>; // assigned in initialSyncWithDOM() + private handleScroll_?: SpecificEventListener<'scroll'>; // assigned in initialSyncWithDOM() private navIcon_!: Element | null; private iconRipples_!: MDCRipple[]; private scrollTarget_!: EventTarget; @@ -57,19 +61,49 @@ export class MDCTopAppBar extends MDCComponent { this.scrollTarget_ = window; } + initialSyncWithDOM() { + this.handleNavigationIconClick_ = this.foundation_.handleNavigationClick.bind(this.foundation_); + this.handleResize_ = this.foundation_.handleResize && this.foundation_.handleResize.bind(this.foundation_); + this.handleScroll_ = this.foundation_.handleScroll && this.foundation_.handleScroll.bind(this.foundation_); + + if (this.navIcon_) { + this.navIcon_.addEventListener('click', this.handleNavigationIconClick_ as EventListener); + } + if (this.handleScroll_) { + this.scrollTarget_.addEventListener('scroll', this.handleScroll_ as EventListener); + } + if (this.handleResize_) { + window.addEventListener('resize', this.handleResize_ as EventListener); + } + } + destroy() { this.iconRipples_.forEach((iconRipple) => iconRipple.destroy()); + if (this.navIcon_) { + this.navIcon_.removeEventListener('click', this.handleNavigationIconClick_ as EventListener); + } + if (this.handleScroll_) { + this.scrollTarget_.removeEventListener('scroll', this.handleScroll_ as EventListener); + } + if (this.handleResize_) { + window.removeEventListener('resize', this.handleResize_ as EventListener); + } super.destroy(); } setScrollTarget(target: EventTarget) { // Remove scroll handler from the previous scroll target - this.foundation_.destroyScrollHandler(); + if (this.handleScroll_) { + this.scrollTarget_.removeEventListener('scroll', this.handleScroll_ as EventListener); + } this.scrollTarget_ = target; // Initialize scroll handler on the new scroll target - this.foundation_.initScrollHandler(); + if (this.handleScroll_) { + this.handleScroll_ = this.foundation_.handleScroll && this.foundation_.handleScroll.bind(this.foundation_); + this.scrollTarget_.addEventListener('scroll', this.handleScroll_ as EventListener); + } } getDefaultFoundation() { @@ -82,21 +116,7 @@ export class MDCTopAppBar extends MDCComponent { removeClass: (className) => this.root_.classList.remove(className), setStyle: (property, value) => (this.root_ as HTMLElement).style.setProperty(property, value), getTopAppBarHeight: () => this.root_.clientHeight, - registerNavigationIconInteractionHandler: (evtType, handler) => { - if (this.navIcon_) { - this.navIcon_.addEventListener(evtType, handler); - } - }, - deregisterNavigationIconInteractionHandler: (evtType, handler) => { - if (this.navIcon_) { - this.navIcon_.removeEventListener(evtType, handler); - } - }, notifyNavigationIconClicked: () => this.emit(strings.NAVIGATION_EVENT, {}), - registerScrollHandler: (handler) => this.scrollTarget_.addEventListener('scroll', handler as EventListener), - deregisterScrollHandler: (handler) => this.scrollTarget_.removeEventListener('scroll', handler as EventListener), - registerResizeHandler: (handler) => window.addEventListener('resize', handler), - deregisterResizeHandler: (handler) => window.removeEventListener('resize', handler), getViewportScrollY: () => { const win = this.scrollTarget_ as Window; const el = this.scrollTarget_ as Element; diff --git a/packages/mdc-top-app-bar/fixed/foundation.ts b/packages/mdc-top-app-bar/fixed/foundation.ts index fcd44b29a55..b833d66b668 100644 --- a/packages/mdc-top-app-bar/fixed/foundation.ts +++ b/packages/mdc-top-app-bar/fixed/foundation.ts @@ -35,7 +35,7 @@ export class MDCFixedTopAppBarFoundation extends MDCTopAppBarFoundation { constructor(adapter?: Partial) { super(adapter); - this.scrollHandler_ = () => this.fixedScrollHandler_(); + this.handleScroll = () => this.fixedScrollHandler_(); } /** diff --git a/packages/mdc-top-app-bar/foundation.ts b/packages/mdc-top-app-bar/foundation.ts index 88dde801950..1f567484a41 100644 --- a/packages/mdc-top-app-bar/foundation.ts +++ b/packages/mdc-top-app-bar/foundation.ts @@ -50,64 +50,23 @@ export class MDCTopAppBarBaseFoundation extends MDCFoundation false, setStyle: () => undefined, getTopAppBarHeight: () => 0, - registerNavigationIconInteractionHandler: () => undefined, - deregisterNavigationIconInteractionHandler: () => undefined, notifyNavigationIconClicked: () => undefined, - registerScrollHandler: () => undefined, - deregisterScrollHandler: () => undefined, - registerResizeHandler: () => undefined, - deregisterResizeHandler: () => undefined, getViewportScrollY: () => 0, getTotalActionItems: () => 0, }; // tslint:enable:object-literal-sort-keys } - protected scrollHandler_?: SpecificEventListener<'scroll'>; - protected resizeHandler_?: SpecificEventListener<'resize'>; - private readonly navClickHandler_: SpecificEventListener<'click'>; + handleScroll?: SpecificEventListener<'scroll'>; + handleResize?: SpecificEventListener<'resize'>; /* istanbul ignore next: optional argument is not a branch statement */ constructor(adapter?: Partial) { super({...MDCTopAppBarBaseFoundation.defaultAdapter, ...adapter}); - - this.navClickHandler_ = () => this.adapter_.notifyNavigationIconClicked(); - } - - init() { - this.initScrollHandler(); - this.initResizeHandler_(); - this.adapter_.registerNavigationIconInteractionHandler('click', this.navClickHandler_); - } - - destroy() { - this.destroyScrollHandler(); - this.destroyResizeHandler_(); - this.adapter_.deregisterNavigationIconInteractionHandler('click', this.navClickHandler_); - } - - initScrollHandler() { - if (this.scrollHandler_) { - this.adapter_.registerScrollHandler(this.scrollHandler_); - } - } - - destroyScrollHandler() { - if (this.scrollHandler_) { - this.adapter_.deregisterScrollHandler(this.scrollHandler_); - } - } - - private initResizeHandler_() { - if (this.resizeHandler_) { - this.adapter_.registerResizeHandler(this.resizeHandler_); - } } - private destroyResizeHandler_() { - if (this.resizeHandler_) { - this.adapter_.deregisterResizeHandler(this.resizeHandler_); - } + handleNavigationClick() { + this.adapter_.notifyNavigationIconClicked(); } } diff --git a/packages/mdc-top-app-bar/short/foundation.ts b/packages/mdc-top-app-bar/short/foundation.ts index 362a899c324..484da1852f2 100644 --- a/packages/mdc-top-app-bar/short/foundation.ts +++ b/packages/mdc-top-app-bar/short/foundation.ts @@ -46,16 +46,13 @@ export class MDCShortTopAppBarFoundation extends MDCTopAppBarBaseFoundation { } if (!this.adapter_.hasClass(cssClasses.SHORT_COLLAPSED_CLASS)) { - this.scrollHandler_ = () => this.shortAppBarScrollHandler_(); - this.adapter_.registerScrollHandler(this.scrollHandler_); + this.handleScroll = () => this.shortAppBarScrollHandler_(); this.shortAppBarScrollHandler_(); + } else { + this.handleScroll = undefined; } } - destroy() { - super.destroy(); - } - /** * Scroll handler for applying/removing the collapsed modifier class on the short top app bar. */ diff --git a/packages/mdc-top-app-bar/standard/foundation.ts b/packages/mdc-top-app-bar/standard/foundation.ts index e0166720cf3..13221d1407e 100644 --- a/packages/mdc-top-app-bar/standard/foundation.ts +++ b/packages/mdc-top-app-bar/standard/foundation.ts @@ -75,8 +75,8 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { this.lastScrollPosition_ = this.adapter_.getViewportScrollY(); this.topAppBarHeight_ = this.adapter_.getTopAppBarHeight(); - this.scrollHandler_ = () => this.topAppBarScrollHandler_(); - this.resizeHandler_ = () => this.topAppBarResizeHandler_(); + this.handleScroll = () => this.topAppBarScrollHandler_(); + this.handleResize = () => this.topAppBarResizeHandler_(); } destroy() { diff --git a/test/unit/mdc-top-app-bar/fixed.foundation.test.js b/test/unit/mdc-top-app-bar/fixed.foundation.test.js index b94a022bc1f..63d12526587 100644 --- a/test/unit/mdc-top-app-bar/fixed.foundation.test.js +++ b/test/unit/mdc-top-app-bar/fixed.foundation.test.js @@ -25,7 +25,6 @@ import td from 'testdouble'; import {MDCFixedTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/fixed/foundation'; import {MDCTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/standard/foundation'; -import {install as installClock} from '../helpers/clock'; suite('MDCFixedTopAppBarFoundation'); @@ -37,68 +36,32 @@ const setupTest = () => { return {foundation, mockAdapter}; }; -const createMockHandlers = (foundation, mockAdapter, clock) => { - let scrollHandler; - td.when(mockAdapter.registerScrollHandler(td.matchers.isA(Function))).thenDo((fn) => { - scrollHandler = fn; - }); - - foundation.init(); - clock.runToFrame(); - td.reset(); - return {scrollHandler}; -}; - -test('fixed top app bar: scroll listener is registered on init', () => { +test('#handleScroll calls #adapter.getViewportScrollY', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.FIXED_CLASS)).thenReturn(true); - foundation.init(); - td.verify(mockAdapter.registerScrollHandler(td.matchers.isA(Function)), {times: 1}); + foundation.handleScroll(); + // twice because it is called once in the standard foundation + td.verify(mockAdapter.getViewportScrollY(), {times: 2}); }); -test('fixed top app bar: scroll listener is removed on destroy', () => { +test('#handleScroll calls #adapter.addClass if adapter.getViewportScrollY > 0', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.FIXED_CLASS)).thenReturn(true); - foundation.init(); - foundation.destroy(); - td.verify(mockAdapter.deregisterScrollHandler(td.matchers.isA(Function)), {times: 1}); -}); - -test('fixed top app bar: class is added once when page is scrolled from the top', () => { - const {foundation, mockAdapter} = setupTest(); - const clock = installClock(); - - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.FIXED_CLASS)).thenReturn(true); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS)).thenReturn(false); - td.when(mockAdapter.getTotalActionItems()).thenReturn(0); - td.when(mockAdapter.getViewportScrollY()).thenReturn(0); - - const {scrollHandler} = createMockHandlers(foundation, mockAdapter, clock); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - - scrollHandler(); - scrollHandler(); - + foundation.handleScroll(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS), {times: 1}); }); -test('fixed top app bar: class is removed once when page is scrolled to the top', () => { +test('#handleScroll calls #adapter.removeClass if adapter.getViewportScrollY < 0 but had just scrolled down', () => { const {foundation, mockAdapter} = setupTest(); - const clock = installClock(); - - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.FIXED_CLASS)).thenReturn(true); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS)).thenReturn(false); - td.when(mockAdapter.getTotalActionItems()).thenReturn(0); - - const {scrollHandler} = createMockHandlers(foundation, mockAdapter, clock); - // Apply the scrolled class td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - scrollHandler(); - - // Test removing it - td.when(mockAdapter.getViewportScrollY()).thenReturn(0); - scrollHandler(); - scrollHandler(); - + foundation.handleScroll(); + td.when(mockAdapter.getViewportScrollY()).thenReturn(-1); + foundation.handleScroll(); td.verify(mockAdapter.removeClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS), {times: 1}); }); + +test('#handleScroll does not call #adapter.removeClass if was not scrolled yet', () => { + const {foundation, mockAdapter} = setupTest(); + td.when(mockAdapter.getViewportScrollY()).thenReturn(-1); + foundation.handleScroll(); + td.verify(mockAdapter.removeClass(td.matchers.isA(String)), {times: 0}); +}); diff --git a/test/unit/mdc-top-app-bar/foundation.test.js b/test/unit/mdc-top-app-bar/foundation.test.js index a9bb276889a..12ebd92cca3 100644 --- a/test/unit/mdc-top-app-bar/foundation.test.js +++ b/test/unit/mdc-top-app-bar/foundation.test.js @@ -23,7 +23,6 @@ import {assert} from 'chai'; import td from 'testdouble'; -import {captureHandlers} from '../helpers/foundation'; import {verifyDefaultAdapter} from '../helpers/foundation'; import {MDCTopAppBarBaseFoundation} from '../../../packages/mdc-top-app-bar/foundation'; @@ -48,10 +47,8 @@ test('exports numbers', () => { test('defaultAdapter returns a complete adapter implementation', () => { verifyDefaultAdapter(MDCTopAppBarBaseFoundation, [ - 'hasClass', 'addClass', 'removeClass', 'setStyle', 'getTopAppBarHeight', 'registerNavigationIconInteractionHandler', - 'deregisterNavigationIconInteractionHandler', 'notifyNavigationIconClicked', 'registerScrollHandler', - 'deregisterScrollHandler', 'registerResizeHandler', 'deregisterResizeHandler', 'getViewportScrollY', - 'getTotalActionItems', + 'hasClass', 'addClass', 'removeClass', 'setStyle', 'getTopAppBarHeight', 'notifyNavigationIconClicked', + 'getViewportScrollY', 'getTotalActionItems', ]); }); @@ -63,31 +60,8 @@ const setupTest = () => { return {foundation, mockAdapter}; }; -test('click on navigation icon emits a navigation event', () => { +test('#handleNavigationClick emits a navigation event', () => { const {foundation, mockAdapter} = setupTest(); - const handlers = captureHandlers(mockAdapter, 'registerNavigationIconInteractionHandler'); - foundation.init(); - handlers.click(); + foundation.handleNavigationClick(); td.verify(mockAdapter.notifyNavigationIconClicked(), {times: 1}); }); - -test('click handler removed from navigation icon during destroy', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.init(); - foundation.destroy(); - td.verify(mockAdapter.deregisterNavigationIconInteractionHandler('click', td.matchers.isA(Function))); -}); - -test('#initScrollHandler calls registerScrollHandler', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.scrollHandler_ = td.func(); - foundation.initScrollHandler(); - td.verify(mockAdapter.registerScrollHandler(foundation.scrollHandler_), {times: 1}); -}); - -test('#destroyScrollHandler calls deregisterScrollHandler', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.scrollHandler_ = td.func(); - foundation.destroyScrollHandler(); - td.verify(mockAdapter.deregisterScrollHandler(foundation.scrollHandler_), {times: 1}); -}); diff --git a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js index 2afd33836c7..f01a583b06e 100644 --- a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js +++ b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js @@ -37,7 +37,7 @@ const MENU_ICONS_COUNT = 3; function getFixture(removeIcon) { const html = bel`
    -
    +
    menu @@ -84,10 +84,15 @@ class FakeRipple { function setupTest(removeIcon = false, rippleFactory = (el) => new FakeRipple(el)) { const fixture = getFixture(removeIcon); const root = fixture.querySelector(strings.ROOT_SELECTOR); + const MockFoundationConstructor = td.constructor(MDCTopAppBarFoundation); + const mockFoundation = new MockFoundationConstructor(); + mockFoundation.handleScroll = td.func(); + mockFoundation.handleResize = td.func(); + const icon = root.querySelector(strings.NAVIGATION_ICON_SELECTOR); - const component = new MDCTopAppBar(root, undefined, rippleFactory); + const component = new MDCTopAppBar(root, mockFoundation, rippleFactory); - return {root, component, icon, fixture}; + return {root, component, icon, mockFoundation, fixture}; } suite('MDCTopAppBar'); @@ -113,6 +118,25 @@ test('constructor does not instantiate ripple for nav icon when not present', () setupTest(/** removeIcon */ true, rippleFactory); }); +test('navIcon click event calls #foundation.handleNavigationClick', () => { + const {root, mockFoundation} = setupTest(); + const navIcon = root.querySelector('.mdc-top-app-bar__navigation-icon'); + domEvents.emit(navIcon, 'click'); + td.verify(mockFoundation.handleNavigationClick(td.matchers.isA(Object)), {times: 1}); +}); + +test('scroll event triggers #foundation.handleScroll', () => { + const {mockFoundation} = setupTest(); + domEvents.emit(window, 'scroll'); + td.verify(mockFoundation.handleScroll(td.matchers.isA(Object)), {times: 1}); +}); + +test('resize event triggers #foundation.handleResize', () => { + const {mockFoundation} = setupTest(); + domEvents.emit(window, 'resize'); + td.verify(mockFoundation.handleResize(td.matchers.isA(Object)), {times: 1}); +}); + test('destroy destroys icon ripples', () => { const {component} = setupTest(); component.destroy(); @@ -121,6 +145,28 @@ test('destroy destroys icon ripples', () => { }); }); +test('destroy destroys scroll event handler', () => { + const {mockFoundation, component} = setupTest(); + component.destroy(); + domEvents.emit(window, 'scroll'); + td.verify(mockFoundation.handleScroll(td.matchers.isA(Object)), {times: 0}); +}); + +test('destroy destroys resize event handler', () => { + const {mockFoundation, component} = setupTest(); + component.destroy(); + domEvents.emit(window, 'resize'); + td.verify(mockFoundation.handleResize(td.matchers.isA(Object)), {times: 0}); +}); + +test('destroy destroys handleNavigationClick handler', () => { + const {mockFoundation, component, root} = setupTest(); + const navIcon = root.querySelector('.mdc-top-app-bar__navigation-icon'); + component.destroy(); + domEvents.emit(navIcon, 'resize'); + td.verify(mockFoundation.handleNavigationClick(td.matchers.isA(Object)), {times: 0}); +}); + test('#setScrollTarget deregisters and registers scroll handler on provided target', () => { const {component} = setupTest(); const fakeTarget1 = document.createElement('div'); @@ -129,14 +175,8 @@ test('#setScrollTarget deregisters and registers scroll handler on provided targ component.setScrollTarget(fakeTarget1); assert.equal(component.scrollTarget_, fakeTarget1); - component.foundation_.destroyScrollHandler = td.func(); - component.foundation_.initScrollHandler = td.func(); - component.setScrollTarget(fakeTarget2); - td.verify(component.foundation_.destroyScrollHandler(), {times: 1}); - td.verify(component.foundation_.initScrollHandler(), {times: 1}); - assert.equal(component.scrollTarget_, fakeTarget2); }); @@ -198,100 +238,6 @@ test('adapter#setStyle sets a style attribute on the root element', () => { assert.isTrue(root.style.getPropertyValue('top') === '1px'); }); -test('registerNavigationIconInteractionHandler does not add a handler to the nav icon if the nav icon is null', () => { - const {component} = setupTest(true); - const handler = td.func('eventHandler'); - - assert.doesNotThrow( - () => component.getDefaultFoundation().adapter_.registerNavigationIconInteractionHandler('click', handler)); -}); - -test('#adapter.registerNavigationIconInteractionHandler adds a handler to the nav icon ' + - 'element for a given event', () => { - const {component, icon} = setupTest(); - const handler = td.func('eventHandler'); - component.getDefaultFoundation().adapter_.registerNavigationIconInteractionHandler('click', handler); - domEvents.emit(icon, 'click'); - td.verify(handler(td.matchers.anything())); -}); - -test('#adapter.deregisterScrollHandler does not remove a handler from the nav icon if the nav icon is null ', () => { - const {component} = setupTest(true); - const handler = td.func('eventHandler'); - - assert.doesNotThrow( - () => component.getDefaultFoundation().adapter_.deregisterNavigationIconInteractionHandler('click', handler)); -}); - -test('#adapter.deregisterNavigationIconInteractionHandler removes a handler from the nav icon ' + - 'element for a given event', () => { - const {component, icon} = setupTest(); - const handler = td.func('eventHandler'); - - icon.addEventListener('click', handler); - component.getDefaultFoundation().adapter_.deregisterNavigationIconInteractionHandler('click', handler); - domEvents.emit(icon, 'click'); - td.verify(handler(td.matchers.anything()), {times: 0}); -}); - -test('#adapter.registerScrollHandler adds a scroll handler to the window ' + - 'element for a given event', () => { - const {component} = setupTest(); - const handler = td.func('scrollHandler'); - component.getDefaultFoundation().adapter_.registerScrollHandler(handler); - - domEvents.emit(window, 'scroll'); - try { - td.verify(handler(td.matchers.anything())); - } finally { - // Just to be safe - window.removeEventListener('scroll', handler); - } -}); - -test('#adapter.deregisterScrollHandler removes a scroll handler from the window ' + - 'element for a given event', () => { - const {component} = setupTest(); - const handler = td.func('scrollHandler'); - window.addEventListener('scroll', handler); - component.getDefaultFoundation().adapter_.deregisterScrollHandler(handler); - domEvents.emit(window, 'scroll'); - try { - td.verify(handler(td.matchers.anything()), {times: 0}); - } finally { - // Just to be safe - window.removeEventListener('scroll', handler); - } -}); - -test('#adapter.registerResizeHandler adds a resize handler to the window', () => { - const {component} = setupTest(); - const handler = td.func('resizeHandler'); - component.getDefaultFoundation().adapter_.registerResizeHandler(handler); - - domEvents.emit(window, 'resize'); - try { - td.verify(handler(td.matchers.anything())); - } finally { - // Just to be safe - window.removeEventListener('resize', handler); - } -}); - -test('#adapter.deregisterResizeHandler removes a resize handler from the window', () => { - const {component} = setupTest(); - const handler = td.func('resizeHandler'); - window.addEventListener('resize', handler); - component.getDefaultFoundation().adapter_.deregisterResizeHandler(handler); - domEvents.emit(window, 'resize'); - try { - td.verify(handler(td.matchers.anything()), {times: 0}); - } finally { - // Just to be safe - window.removeEventListener('resize', handler); - } -}); - test('adapter#getViewportScrollY returns scroll distance', () => { const {component} = setupTest(); assert.equal(component.getDefaultFoundation().adapter_.getViewportScrollY(), window.pageYOffset); diff --git a/test/unit/mdc-top-app-bar/short.foundation.test.js b/test/unit/mdc-top-app-bar/short.foundation.test.js index 98b97135a0a..1c944d4886f 100644 --- a/test/unit/mdc-top-app-bar/short.foundation.test.js +++ b/test/unit/mdc-top-app-bar/short.foundation.test.js @@ -22,107 +22,76 @@ */ import td from 'testdouble'; +import {assert} from 'chai'; import {MDCShortTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/short/foundation'; import {MDCTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/standard/foundation'; -import {install as installClock} from '../helpers/clock'; suite('MDCShortTopAppBarFoundation'); const setupTest = () => { const mockAdapter = td.object(MDCTopAppBarFoundation.defaultAdapter); + td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); + const foundation = new MDCShortTopAppBarFoundation(mockAdapter); return {foundation, mockAdapter}; }; -const createMockHandlers = (foundation, mockAdapter, clock) => { - let scrollHandler; - td.when(mockAdapter.registerScrollHandler(td.matchers.isA(Function))).thenDo((fn) => { - scrollHandler = fn; - }); - - foundation.init(); - clock.runToFrame(); - td.reset(); - return {scrollHandler}; -}; - -test('short top app bar: scroll listener is registered on init', () => { +test('short top app bar: scrollHandler calls #adapter.getViewportScrollY', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); - foundation.init(); - td.verify(mockAdapter.registerScrollHandler(td.matchers.isA(Function)), {times: 1}); -}); - -test('short top app bar: scroll listener is removed on destroy', () => { - const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); foundation.init(); - foundation.destroy(); - td.verify(mockAdapter.deregisterScrollHandler(td.matchers.isA(Function)), {times: 1}); + foundation.handleScroll(); + // called twice because its called once in the standard foundation + td.verify(mockAdapter.getViewportScrollY(), {times: 2}); }); -test('short top app bar: scroll listener is not registered if collapsed class exists before init', () => { +test('short top app bar: scrollHandler does not exist if collapsed class exists before init', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS)).thenReturn(true); - td.when(mockAdapter.getTotalActionItems()).thenReturn(0); foundation.init(); - td.verify(mockAdapter.registerScrollHandler(td.matchers.anything()), {times: 0}); + assert.equal(foundation.handleScroll, undefined); }); -test('short top app bar: class is added once when page is scrolled from the top', () => { +test('short top app bar: #adapter.addClass called when page is scrolled from the top', () => { const {foundation, mockAdapter} = setupTest(); - const clock = installClock(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS)).thenReturn(false); - td.when(mockAdapter.getTotalActionItems()).thenReturn(0); - td.when(mockAdapter.getViewportScrollY()).thenReturn(0); - - const {scrollHandler} = createMockHandlers(foundation, mockAdapter, clock); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - - scrollHandler(); - scrollHandler(); + foundation.init(); + foundation.handleScroll(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS), {times: 1}); }); -test('short top app bar: class is removed once when page is scrolled to the top', () => { +test('short top app bar: #adapter.removeClass called when page is scrolled to the top', () => { const {foundation, mockAdapter} = setupTest(); - const clock = installClock(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS)).thenReturn(false); td.when(mockAdapter.getTotalActionItems()).thenReturn(0); + foundation.init(); - const {scrollHandler} = createMockHandlers(foundation, mockAdapter, clock); // Apply the collapsed class td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - scrollHandler(); + foundation.handleScroll(); // Test removing it td.when(mockAdapter.getViewportScrollY()).thenReturn(0); - scrollHandler(); - scrollHandler(); + foundation.handleScroll(); td.verify(mockAdapter.removeClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS), {times: 1}); }); -test('short top app bar: class is added if it has an action item', () => { +test('short top app bar: #adapter.addClass is called if it has an action item', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); td.when(mockAdapter.getTotalActionItems()).thenReturn(1); foundation.init(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.SHORT_HAS_ACTION_ITEM_CLASS), {times: 1}); }); -test('short top app bar: class is not added if it does not have an action item', () => { +test('short top app bar: #adapter.addClass is not called if it does not have an action item', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); td.when(mockAdapter.getTotalActionItems()).thenReturn(0); foundation.init(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.SHORT_HAS_ACTION_ITEM_CLASS), {times: 0}); diff --git a/test/unit/mdc-top-app-bar/standard.foundation.test.js b/test/unit/mdc-top-app-bar/standard.foundation.test.js index be8ba4fde5b..f09d88b6ec2 100644 --- a/test/unit/mdc-top-app-bar/standard.foundation.test.js +++ b/test/unit/mdc-top-app-bar/standard.foundation.test.js @@ -40,41 +40,8 @@ const setupTest = () => { return {foundation, mockAdapter}; }; -const createMockHandlers = (foundation, mockAdapter, clock) => { - let scrollHandler; - let resizeHandler; - td.when(mockAdapter.registerScrollHandler(td.matchers.isA(Function))).thenDo((fn) => { - scrollHandler = fn; - }); - td.when(mockAdapter.registerResizeHandler(td.matchers.isA(Function))).thenDo((fn) => { - resizeHandler = fn; - }); - - foundation.init(); - clock.runToFrame(); - td.reset(); - return {scrollHandler, resizeHandler}; -}; - -test('top app bar: listeners is registered on init', () => { - const {foundation, mockAdapter} = setupTest(); - foundation.init(); - td.verify(mockAdapter.registerScrollHandler(td.matchers.isA(Function)), {times: 1}); - td.verify(mockAdapter.registerResizeHandler(td.matchers.isA(Function)), {times: 1}); -}); - -test('listeners removed on destroy', () => { - const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_CLASS)).thenReturn(true); - foundation.init(); - foundation.destroy(); - td.verify(mockAdapter.deregisterScrollHandler(td.matchers.isA(Function)), {times: 1}); - td.verify(mockAdapter.deregisterResizeHandler(td.matchers.isA(Function)), {times: 1}); -}); - test('top app bar scroll: throttledResizeHandler_ updates topAppBarHeight_ if the top app bar height changes', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); assert.isTrue(foundation.topAppBarHeight_ === 64); td.when(mockAdapter.getTopAppBarHeight()).thenReturn(56); foundation.throttledResizeHandler_(); @@ -83,7 +50,6 @@ test('top app bar scroll: throttledResizeHandler_ updates topAppBarHeight_ if th test('top app bar scroll: throttledResizeHandler_ does not update topAppBarHeight_ if height is the same', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); assert.isTrue(foundation.topAppBarHeight_ === 64); td.when(mockAdapter.getTopAppBarHeight()).thenReturn(64); foundation.throttledResizeHandler_(); @@ -92,7 +58,6 @@ test('top app bar scroll: throttledResizeHandler_ does not update topAppBarHeigh test('top app bar : moveTopAppBar_ update required transition from fully shown to 1px scrolled', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = -1; // Indicates 1px scrolled up foundation.checkForUpdate_ = () => true; foundation.moveTopAppBar_(); @@ -101,7 +66,6 @@ test('top app bar : moveTopAppBar_ update required transition from fully shown t test('top app bar : moveTopAppBar_ update required transition from 1px shown to fullyHidden ', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = -64; // Indicates 64px scrolled foundation.checkForUpdate_ = () => true; foundation.moveTopAppBar_(); @@ -110,7 +74,6 @@ test('top app bar : moveTopAppBar_ update required transition from 1px shown to test('top app bar : moveTopAppBar_ update is not required results in no top app bar style change', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = 0; foundation.checkForUpdate_ = () => false; foundation.moveTopAppBar_(); @@ -119,7 +82,6 @@ test('top app bar : moveTopAppBar_ update is not required results in no top app test('top app bar : checkForUpdate_ returns true if top app bar is not docked', () => { const {foundation} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = -1; foundation.wasDocked_ = false; assert.isTrue(foundation.checkForUpdate_()); @@ -127,7 +89,6 @@ test('top app bar : checkForUpdate_ returns true if top app bar is not docked', test('top app bar : checkForUpdate_ updates wasDocked_ to true if top app bar becomes docked', () => { const {foundation} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = 0; foundation.wasDocked_ = false; assert.isTrue(foundation.checkForUpdate_()); @@ -136,7 +97,6 @@ test('top app bar : checkForUpdate_ updates wasDocked_ to true if top app bar be test('top app bar : checkForUpdate_ returns false if top app bar is docked and fullyShown', () => { const {foundation} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = 0; foundation.wasDocked_ = true; assert.isFalse(foundation.checkForUpdate_()); @@ -145,7 +105,6 @@ test('top app bar : checkForUpdate_ returns false if top app bar is docked and f test('top app bar : checkForUpdate_ returns false if top app bar is docked and fullyHidden', () => { const {foundation} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = -64; foundation.wasDocked_ = true; foundation.isDockedShowing_ = false; @@ -155,93 +114,79 @@ test('top app bar : checkForUpdate_ returns false if top app bar is docked and f test('top app bar : checkForUpdate_ returns true if top app bar is docked but not fullyShown/fullyHidden', () => { const {foundation} = setupTest(); - foundation.init(); foundation.currentAppBarOffsetTop_ = -63; foundation.wasDocked_ = true; assert.isTrue(foundation.checkForUpdate_()); assert.isFalse(foundation.wasDocked_); }); -test('top app bar : topAppBarScrollHandler_ does not update currentAppBarOffsetTop_ if ' + +test('top app bar : handleScroll does not update currentAppBarOffsetTop_ if ' + 'isCurrentlyBeingResized_ is true', () => { const {foundation} = setupTest(); - foundation.init(); foundation.isCurrentlyBeingResized_ = true; - foundation.topAppBarScrollHandler_(); + foundation.handleScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === 0); }); -test('top app bar : topAppBarScrollHandler_ subtracts the currentAppBarOffsetTop_ amount scrolled', () => { +test('top app bar : handleScroll subtracts the currentAppBarOffsetTop_ amount scrolled', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - foundation.topAppBarScrollHandler_(); + foundation.handleScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === -1); }); -test('top app bar : topAppBarScrollHandler_ negative scroll results in currentAppBarOffsetTop_ being 0', () => { +test('top app bar : handleScroll negative scroll results in currentAppBarOffsetTop_ being 0', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); td.when(mockAdapter.getViewportScrollY()).thenReturn(-1); - foundation.topAppBarScrollHandler_(); + foundation.handleScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === 0); }); -test('top app bar : topAppBarScrollHandler_ scroll greater than top app bar height results in ' + +test('top app bar : handleScroll scroll greater than top app bar height results in ' + 'currentAppBarOffsetTop_ being negative topAppBarHeight_', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); td.when(mockAdapter.getViewportScrollY()).thenReturn(100); - foundation.topAppBarScrollHandler_(); + foundation.handleScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === -64); }); -test('top app bar : topAppBarScrollHandler_ scrolling does not generate a ' + +test('top app bar : handleScroll scrolling does not generate a ' + 'positive currentAppBarOffsetTop_', () => { const {foundation, mockAdapter} = setupTest(); - foundation.init(); td.when(mockAdapter.getViewportScrollY()).thenReturn(100); - foundation.topAppBarScrollHandler_(); + foundation.handleScroll(); td.when(mockAdapter.getViewportScrollY()).thenReturn(-100); - foundation.topAppBarScrollHandler_(); + foundation.handleScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === 0); }); test('top app bar : resize events should set isCurrentlyBeingResized_ to true', () => { - const clock = installClock(); - const {foundation, mockAdapter} = setupTest(); - const {resizeHandler} = createMockHandlers(foundation, mockAdapter, clock); + const {foundation} = setupTest(); - foundation.init(); - resizeHandler(); + foundation.handleResize(); assert.isTrue(foundation.isCurrentlyBeingResized_); }); test('top app bar : resize events throttle multiple calls of throttledResizeHandler_ ', () => { const clock = installClock(); - const {foundation, mockAdapter} = setupTest(); - const {resizeHandler} = createMockHandlers(foundation, mockAdapter, clock); + const {foundation} = setupTest(); - foundation.init(); - resizeHandler(); + foundation.handleResize(); assert.isFalse(!foundation.resizeThrottleId_); - resizeHandler(); + foundation.handleResize(); clock.tick(numbers.DEBOUNCE_THROTTLE_RESIZE_TIME_MS); assert.isTrue(!foundation.resizeThrottleId_); }); test('top app bar : resize events debounce changing isCurrentlyBeingResized_ to false ', () => { const clock = installClock(); - const {foundation, mockAdapter} = setupTest(); - const {resizeHandler} = createMockHandlers(foundation, mockAdapter, clock); - - foundation.init(); + const {foundation} = setupTest(); - resizeHandler(); + foundation.handleResize(); const debounceId = foundation.resizeDebounceId_; clock.tick(50); - resizeHandler(); + foundation.handleResize(); assert.isFalse(debounceId === foundation.resizeDebounceId_); assert.isTrue(foundation.isCurrentlyBeingResized_); clock.tick(150); From 4c056e1bc099449f2db5e7fb4e30837c7caea551 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Tue, 14 May 2019 11:52:24 -0700 Subject: [PATCH 11/17] fix: update tests --- package-lock.json | 41 +++++--- packages/mdc-top-app-bar/README.md | 4 +- packages/mdc-top-app-bar/component.ts | 45 +++++---- packages/mdc-top-app-bar/fixed/foundation.ts | 10 +- packages/mdc-top-app-bar/foundation.ts | 4 +- packages/mdc-top-app-bar/short/foundation.ts | 10 +- .../mdc-top-app-bar/standard/foundation.ts | 95 +++++++++---------- .../mdc-top-app-bar/fixed.foundation.test.js | 19 ++-- .../mdc-top-app-bar/mdc-top-app-bar.test.js | 16 ++-- .../mdc-top-app-bar/short.foundation.test.js | 14 +-- .../standard.foundation.test.js | 32 +++---- 11 files changed, 151 insertions(+), 139 deletions(-) diff --git a/package-lock.json b/package-lock.json index 54d89902eb9..08729cd5683 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8244,7 +8244,8 @@ "version": "2.1.1", "resolved": false, "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=", - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -8268,13 +8269,15 @@ "version": "1.0.0", "resolved": false, "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=", - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "resolved": false, "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -8291,19 +8294,22 @@ "version": "1.1.0", "resolved": false, "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=", - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "resolved": false, "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=", - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "resolved": false, "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=", - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -8434,7 +8440,8 @@ "version": "2.0.3", "resolved": false, "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -8448,6 +8455,7 @@ "resolved": false, "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -8464,6 +8472,7 @@ "resolved": false, "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -8472,13 +8481,15 @@ "version": "0.0.8", "resolved": false, "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.3.5", "resolved": false, "integrity": "sha512-Gi1W4k059gyRbyVUZQ4mEqLm0YIUiGYfvxhF6SIlk3ui1WVxMTGfGdQ2SInh3PDrRTVvPKgULkpJtT4RH10+VA==", "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -8499,6 +8510,7 @@ "resolved": false, "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -8587,7 +8599,8 @@ "version": "1.0.1", "resolved": false, "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=", - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -8601,6 +8614,7 @@ "resolved": false, "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -8696,7 +8710,8 @@ "version": "5.1.2", "resolved": false, "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==", - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -8738,6 +8753,7 @@ "resolved": false, "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -8759,6 +8775,7 @@ "resolved": false, "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -8807,13 +8824,15 @@ "version": "1.0.2", "resolved": false, "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=", - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.3", "resolved": false, "integrity": "sha512-S+Zk8DEWE6oKpV+vI3qWkaK+jSbIK86pCwe2IF/xwIpQ8jEuxpw9NyaGjmp9+BoJv5FV2piqCDcoCtStppiq2A==", - "dev": true + "dev": true, + "optional": true } } }, diff --git a/packages/mdc-top-app-bar/README.md b/packages/mdc-top-app-bar/README.md index 533cab6746b..35c56bbd961 100644 --- a/packages/mdc-top-app-bar/README.md +++ b/packages/mdc-top-app-bar/README.md @@ -228,8 +228,8 @@ All foundations provide the following methods: Method Signature | Description --- | --- -`handleScroll() => void` | Handles `scroll` event on specified scrollTarget (default to `window`). -`handleResize() => void` | Handles `resize` event on window. +`handleTargetScroll() => void` | Handles `scroll` event on specified scrollTarget (default to `window`). +`handleWindowResize() => void` | Handles `resize` event on window. `handleNavigationClick() => void` | Handles `click` event on navigation icon. #### `MDCShortTopAppBarFoundation` diff --git a/packages/mdc-top-app-bar/component.ts b/packages/mdc-top-app-bar/component.ts index 17c9bdf9048..8a38d9487b6 100644 --- a/packages/mdc-top-app-bar/component.ts +++ b/packages/mdc-top-app-bar/component.ts @@ -36,9 +36,9 @@ export class MDCTopAppBar extends MDCComponent { return new MDCTopAppBar(root); } - private handleNavigationIconClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() - private handleResize_?: SpecificEventListener<'resize'>; // assigned in initialSyncWithDOM() - private handleScroll_?: SpecificEventListener<'scroll'>; // assigned in initialSyncWithDOM() + private handleNavigationClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() + private handleWindowResize_?: SpecificEventListener<'resize'>; // assigned in initialSyncWithDOM() + private handleTargetScroll_?: SpecificEventListener<'scroll'>; // assigned in initialSyncWithDOM() private navIcon_!: Element | null; private iconRipples_!: MDCRipple[]; private scrollTarget_!: EventTarget; @@ -62,47 +62,50 @@ export class MDCTopAppBar extends MDCComponent { } initialSyncWithDOM() { - this.handleNavigationIconClick_ = this.foundation_.handleNavigationClick.bind(this.foundation_); - this.handleResize_ = this.foundation_.handleResize && this.foundation_.handleResize.bind(this.foundation_); - this.handleScroll_ = this.foundation_.handleScroll && this.foundation_.handleScroll.bind(this.foundation_); + this.handleNavigationClick_ = this.foundation_.handleNavigationClick.bind(this.foundation_); + this.handleWindowResize_ + = this.foundation_.handleWindowResize && this.foundation_.handleWindowResize.bind(this.foundation_); + this.handleTargetScroll_ + = this.foundation_.handleTargetScroll && this.foundation_.handleTargetScroll.bind(this.foundation_); if (this.navIcon_) { - this.navIcon_.addEventListener('click', this.handleNavigationIconClick_ as EventListener); + this.navIcon_.addEventListener('click', this.handleNavigationClick_ as EventListener); } - if (this.handleScroll_) { - this.scrollTarget_.addEventListener('scroll', this.handleScroll_ as EventListener); + if (this.handleTargetScroll_) { + this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); } - if (this.handleResize_) { - window.addEventListener('resize', this.handleResize_ as EventListener); + if (this.handleWindowResize_) { + window.addEventListener('resize', this.handleWindowResize_ as EventListener); } } destroy() { this.iconRipples_.forEach((iconRipple) => iconRipple.destroy()); if (this.navIcon_) { - this.navIcon_.removeEventListener('click', this.handleNavigationIconClick_ as EventListener); + this.navIcon_.removeEventListener('click', this.handleNavigationClick_ as EventListener); } - if (this.handleScroll_) { - this.scrollTarget_.removeEventListener('scroll', this.handleScroll_ as EventListener); + if (this.handleTargetScroll_) { + this.scrollTarget_.removeEventListener('scroll', this.handleTargetScroll_ as EventListener); } - if (this.handleResize_) { - window.removeEventListener('resize', this.handleResize_ as EventListener); + if (this.handleWindowResize_) { + window.removeEventListener('resize', this.handleWindowResize_ as EventListener); } super.destroy(); } setScrollTarget(target: EventTarget) { // Remove scroll handler from the previous scroll target - if (this.handleScroll_) { - this.scrollTarget_.removeEventListener('scroll', this.handleScroll_ as EventListener); + if (this.handleTargetScroll_) { + this.scrollTarget_.removeEventListener('scroll', this.handleTargetScroll_ as EventListener); } this.scrollTarget_ = target; // Initialize scroll handler on the new scroll target - if (this.handleScroll_) { - this.handleScroll_ = this.foundation_.handleScroll && this.foundation_.handleScroll.bind(this.foundation_); - this.scrollTarget_.addEventListener('scroll', this.handleScroll_ as EventListener); + if (this.handleTargetScroll_) { + this.handleTargetScroll_ + = this.foundation_.handleTargetScroll && this.foundation_.handleTargetScroll.bind(this.foundation_); + this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); } } diff --git a/packages/mdc-top-app-bar/fixed/foundation.ts b/packages/mdc-top-app-bar/fixed/foundation.ts index b833d66b668..76f96a98a10 100644 --- a/packages/mdc-top-app-bar/fixed/foundation.ts +++ b/packages/mdc-top-app-bar/fixed/foundation.ts @@ -21,7 +21,6 @@ * THE SOFTWARE. */ -import {MDCTopAppBarAdapter} from '../adapter'; import {cssClasses} from '../constants'; import {MDCTopAppBarFoundation} from '../standard/foundation'; @@ -31,17 +30,10 @@ export class MDCFixedTopAppBarFoundation extends MDCTopAppBarFoundation { */ private wasScrolled_ = false; - /* istanbul ignore next: optional argument is not a branch statement */ - constructor(adapter?: Partial) { - super(adapter); - - this.handleScroll = () => this.fixedScrollHandler_(); - } - /** * Scroll handler for applying/removing the modifier class on the fixed top app bar. */ - private fixedScrollHandler_() { + handleTargetScroll = () => { const currentScroll = this.adapter_.getViewportScrollY(); if (currentScroll <= 0) { diff --git a/packages/mdc-top-app-bar/foundation.ts b/packages/mdc-top-app-bar/foundation.ts index 1f567484a41..65d752bf511 100644 --- a/packages/mdc-top-app-bar/foundation.ts +++ b/packages/mdc-top-app-bar/foundation.ts @@ -57,8 +57,8 @@ export class MDCTopAppBarBaseFoundation extends MDCFoundation; - handleResize?: SpecificEventListener<'resize'>; + handleTargetScroll?: SpecificEventListener<'scroll'>; + handleWindowResize?: SpecificEventListener<'resize'>; /* istanbul ignore next: optional argument is not a branch statement */ constructor(adapter?: Partial) { diff --git a/packages/mdc-top-app-bar/short/foundation.ts b/packages/mdc-top-app-bar/short/foundation.ts index 484da1852f2..e886433ecc4 100644 --- a/packages/mdc-top-app-bar/short/foundation.ts +++ b/packages/mdc-top-app-bar/short/foundation.ts @@ -46,17 +46,17 @@ export class MDCShortTopAppBarFoundation extends MDCTopAppBarBaseFoundation { } if (!this.adapter_.hasClass(cssClasses.SHORT_COLLAPSED_CLASS)) { - this.handleScroll = () => this.shortAppBarScrollHandler_(); - this.shortAppBarScrollHandler_(); - } else { - this.handleScroll = undefined; + this.handleTargetScroll(); } } /** * Scroll handler for applying/removing the collapsed modifier class on the short top app bar. */ - private shortAppBarScrollHandler_() { + handleTargetScroll = () => { + if (this.adapter_.hasClass(cssClasses.SHORT_COLLAPSED_CLASS)) { + return; + } const currentScroll = this.adapter_.getViewportScrollY(); if (currentScroll <= 0) { diff --git a/packages/mdc-top-app-bar/standard/foundation.ts b/packages/mdc-top-app-bar/standard/foundation.ts index 13221d1407e..9e8b0941649 100644 --- a/packages/mdc-top-app-bar/standard/foundation.ts +++ b/packages/mdc-top-app-bar/standard/foundation.ts @@ -74,9 +74,6 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { this.lastScrollPosition_ = this.adapter_.getViewportScrollY(); this.topAppBarHeight_ = this.adapter_.getTopAppBarHeight(); - - this.handleScroll = () => this.topAppBarScrollHandler_(); - this.handleResize = () => this.topAppBarResizeHandler_(); } destroy() { @@ -84,52 +81,10 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { this.adapter_.setStyle('top', ''); } - /** - * Function to determine if the DOM needs to update. - */ - private checkForUpdate_(): boolean { - const offscreenBoundaryTop = -this.topAppBarHeight_; - const hasAnyPixelsOffscreen = this.currentAppBarOffsetTop_ < 0; - const hasAnyPixelsOnscreen = this.currentAppBarOffsetTop_ > offscreenBoundaryTop; - const partiallyShowing = hasAnyPixelsOffscreen && hasAnyPixelsOnscreen; - - // If it's partially showing, it can't be docked. - if (partiallyShowing) { - this.wasDocked_ = false; - } else { - // Not previously docked and not partially showing, it's now docked. - if (!this.wasDocked_) { - this.wasDocked_ = true; - return true; - } else if (this.isDockedShowing_ !== hasAnyPixelsOnscreen) { - this.isDockedShowing_ = hasAnyPixelsOnscreen; - return true; - } - } - - return partiallyShowing; - } - - /** - * Function to move the top app bar if needed. - */ - private moveTopAppBar_() { - if (this.checkForUpdate_()) { - // Once the top app bar is fully hidden we use the max potential top app bar height as our offset - // so the top app bar doesn't show if the window resizes and the new height > the old height. - let offset = this.currentAppBarOffsetTop_; - if (Math.abs(offset) >= this.topAppBarHeight_) { - offset = -numbers.MAX_TOP_APP_BAR_HEIGHT; - } - - this.adapter_.setStyle('top', offset + 'px'); - } - } - /** * Scroll handler for the default scroll behavior of the top app bar. */ - private topAppBarScrollHandler_() { + handleTargetScroll = () => { const currentScrollPosition = Math.max(this.adapter_.getViewportScrollY(), 0); const diff = currentScrollPosition - this.lastScrollPosition_; this.lastScrollPosition_ = currentScrollPosition; @@ -152,7 +107,7 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { /** * Top app bar resize handler that throttle/debounce functions that execute updates. */ - private topAppBarResizeHandler_() { + handleWindowResize = () => { // Throttle resize events 10 p/s if (!this.resizeThrottleId_) { this.resizeThrottleId_ = setTimeout(() => { @@ -168,12 +123,54 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { } this.resizeDebounceId_ = setTimeout(() => { - this.topAppBarScrollHandler_(); + this.handleTargetScroll(); this.isCurrentlyBeingResized_ = false; this.resizeDebounceId_ = INITIAL_VALUE; }, numbers.DEBOUNCE_THROTTLE_RESIZE_TIME_MS); } + /** + * Function to determine if the DOM needs to update. + */ + private checkForUpdate_(): boolean { + const offscreenBoundaryTop = -this.topAppBarHeight_; + const hasAnyPixelsOffscreen = this.currentAppBarOffsetTop_ < 0; + const hasAnyPixelsOnscreen = this.currentAppBarOffsetTop_ > offscreenBoundaryTop; + const partiallyShowing = hasAnyPixelsOffscreen && hasAnyPixelsOnscreen; + + // If it's partially showing, it can't be docked. + if (partiallyShowing) { + this.wasDocked_ = false; + } else { + // Not previously docked and not partially showing, it's now docked. + if (!this.wasDocked_) { + this.wasDocked_ = true; + return true; + } else if (this.isDockedShowing_ !== hasAnyPixelsOnscreen) { + this.isDockedShowing_ = hasAnyPixelsOnscreen; + return true; + } + } + + return partiallyShowing; + } + + /** + * Function to move the top app bar if needed. + */ + private moveTopAppBar_() { + if (this.checkForUpdate_()) { + // Once the top app bar is fully hidden we use the max potential top app bar height as our offset + // so the top app bar doesn't show if the window resizes and the new height > the old height. + let offset = this.currentAppBarOffsetTop_; + if (Math.abs(offset) >= this.topAppBarHeight_) { + offset = -numbers.MAX_TOP_APP_BAR_HEIGHT; + } + + this.adapter_.setStyle('top', offset + 'px'); + } + } + /** * Throttled function that updates the top app bar scrolled values if the * top app bar height changes. @@ -189,7 +186,7 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { this.currentAppBarOffsetTop_ -= this.topAppBarHeight_ - currentHeight; this.topAppBarHeight_ = currentHeight; } - this.topAppBarScrollHandler_(); + this.handleTargetScroll(); } } diff --git a/test/unit/mdc-top-app-bar/fixed.foundation.test.js b/test/unit/mdc-top-app-bar/fixed.foundation.test.js index 63d12526587..60cda84ed52 100644 --- a/test/unit/mdc-top-app-bar/fixed.foundation.test.js +++ b/test/unit/mdc-top-app-bar/fixed.foundation.test.js @@ -36,32 +36,33 @@ const setupTest = () => { return {foundation, mockAdapter}; }; -test('#handleScroll calls #adapter.getViewportScrollY', () => { +test('#handleTargetScroll calls #adapter.getViewportScrollY', () => { const {foundation, mockAdapter} = setupTest(); - foundation.handleScroll(); + foundation.handleTargetScroll(); // twice because it is called once in the standard foundation td.verify(mockAdapter.getViewportScrollY(), {times: 2}); }); -test('#handleScroll calls #adapter.addClass if adapter.getViewportScrollY > 0', () => { +test('#handleTargetScroll calls #adapter.addClass if adapter.getViewportScrollY > 0', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS), {times: 1}); }); -test('#handleScroll calls #adapter.removeClass if adapter.getViewportScrollY < 0 but had just scrolled down', () => { +test('#handleTargetScroll calls #adapter.removeClass if ' + +'adapter.getViewportScrollY < 0 but had just scrolled down', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.when(mockAdapter.getViewportScrollY()).thenReturn(-1); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.verify(mockAdapter.removeClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS), {times: 1}); }); -test('#handleScroll does not call #adapter.removeClass if was not scrolled yet', () => { +test('#handleTargetScroll does not call #adapter.removeClass if was not scrolled yet', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(-1); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.verify(mockAdapter.removeClass(td.matchers.isA(String)), {times: 0}); }); diff --git a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js index f01a583b06e..1bf2f3aa095 100644 --- a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js +++ b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js @@ -86,8 +86,8 @@ function setupTest(removeIcon = false, rippleFactory = (el) => new FakeRipple(el const root = fixture.querySelector(strings.ROOT_SELECTOR); const MockFoundationConstructor = td.constructor(MDCTopAppBarFoundation); const mockFoundation = new MockFoundationConstructor(); - mockFoundation.handleScroll = td.func(); - mockFoundation.handleResize = td.func(); + mockFoundation.handleTargetScroll = td.func(); + mockFoundation.handleWindowResize = td.func(); const icon = root.querySelector(strings.NAVIGATION_ICON_SELECTOR); const component = new MDCTopAppBar(root, mockFoundation, rippleFactory); @@ -125,16 +125,16 @@ test('navIcon click event calls #foundation.handleNavigationClick', () => { td.verify(mockFoundation.handleNavigationClick(td.matchers.isA(Object)), {times: 1}); }); -test('scroll event triggers #foundation.handleScroll', () => { +test('scroll event triggers #foundation.handleTargetScroll', () => { const {mockFoundation} = setupTest(); domEvents.emit(window, 'scroll'); - td.verify(mockFoundation.handleScroll(td.matchers.isA(Object)), {times: 1}); + td.verify(mockFoundation.handleTargetScroll(td.matchers.isA(Object)), {times: 1}); }); -test('resize event triggers #foundation.handleResize', () => { +test('resize event triggers #foundation.handleWindowResize', () => { const {mockFoundation} = setupTest(); domEvents.emit(window, 'resize'); - td.verify(mockFoundation.handleResize(td.matchers.isA(Object)), {times: 1}); + td.verify(mockFoundation.handleWindowResize(td.matchers.isA(Object)), {times: 1}); }); test('destroy destroys icon ripples', () => { @@ -149,14 +149,14 @@ test('destroy destroys scroll event handler', () => { const {mockFoundation, component} = setupTest(); component.destroy(); domEvents.emit(window, 'scroll'); - td.verify(mockFoundation.handleScroll(td.matchers.isA(Object)), {times: 0}); + td.verify(mockFoundation.handleTargetScroll(td.matchers.isA(Object)), {times: 0}); }); test('destroy destroys resize event handler', () => { const {mockFoundation, component} = setupTest(); component.destroy(); domEvents.emit(window, 'resize'); - td.verify(mockFoundation.handleResize(td.matchers.isA(Object)), {times: 0}); + td.verify(mockFoundation.handleWindowResize(td.matchers.isA(Object)), {times: 0}); }); test('destroy destroys handleNavigationClick handler', () => { diff --git a/test/unit/mdc-top-app-bar/short.foundation.test.js b/test/unit/mdc-top-app-bar/short.foundation.test.js index 1c944d4886f..8dc5a86d5e1 100644 --- a/test/unit/mdc-top-app-bar/short.foundation.test.js +++ b/test/unit/mdc-top-app-bar/short.foundation.test.js @@ -22,7 +22,6 @@ */ import td from 'testdouble'; -import {assert} from 'chai'; import {MDCShortTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/short/foundation'; import {MDCTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/standard/foundation'; @@ -42,16 +41,17 @@ const setupTest = () => { test('short top app bar: scrollHandler calls #adapter.getViewportScrollY', () => { const {foundation, mockAdapter} = setupTest(); foundation.init(); - foundation.handleScroll(); + foundation.handleTargetScroll(); // called twice because its called once in the standard foundation td.verify(mockAdapter.getViewportScrollY(), {times: 2}); }); -test('short top app bar: scrollHandler does not exist if collapsed class exists before init', () => { +test('short top app bar: scrollHandler does not call getViewportScrollY method ' + +'if short collapsed class is on the component', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS)).thenReturn(true); foundation.init(); - assert.equal(foundation.handleScroll, undefined); + td.verify(mockAdapter.getViewportScrollY(), {times: 0}); }); test('short top app bar: #adapter.addClass called when page is scrolled from the top', () => { @@ -60,7 +60,7 @@ test('short top app bar: #adapter.addClass called when page is scrolled from the td.when(mockAdapter.hasClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS)).thenReturn(false); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); foundation.init(); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS), {times: 1}); }); @@ -74,11 +74,11 @@ test('short top app bar: #adapter.removeClass called when page is scrolled to th // Apply the collapsed class td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - foundation.handleScroll(); + foundation.handleTargetScroll(); // Test removing it td.when(mockAdapter.getViewportScrollY()).thenReturn(0); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.verify(mockAdapter.removeClass(MDCTopAppBarFoundation.cssClasses.SHORT_COLLAPSED_CLASS), {times: 1}); }); diff --git a/test/unit/mdc-top-app-bar/standard.foundation.test.js b/test/unit/mdc-top-app-bar/standard.foundation.test.js index f09d88b6ec2..206ab763d9b 100644 --- a/test/unit/mdc-top-app-bar/standard.foundation.test.js +++ b/test/unit/mdc-top-app-bar/standard.foundation.test.js @@ -120,50 +120,50 @@ test('top app bar : checkForUpdate_ returns true if top app bar is docked but no assert.isFalse(foundation.wasDocked_); }); -test('top app bar : handleScroll does not update currentAppBarOffsetTop_ if ' + +test('top app bar : handleTargetScroll does not update currentAppBarOffsetTop_ if ' + 'isCurrentlyBeingResized_ is true', () => { const {foundation} = setupTest(); foundation.isCurrentlyBeingResized_ = true; - foundation.handleScroll(); + foundation.handleTargetScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === 0); }); -test('top app bar : handleScroll subtracts the currentAppBarOffsetTop_ amount scrolled', () => { +test('top app bar : handleTargetScroll subtracts the currentAppBarOffsetTop_ amount scrolled', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(1); - foundation.handleScroll(); + foundation.handleTargetScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === -1); }); -test('top app bar : handleScroll negative scroll results in currentAppBarOffsetTop_ being 0', () => { +test('top app bar : handleTargetScroll negative scroll results in currentAppBarOffsetTop_ being 0', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(-1); - foundation.handleScroll(); + foundation.handleTargetScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === 0); }); -test('top app bar : handleScroll scroll greater than top app bar height results in ' + +test('top app bar : handleTargetScroll scroll greater than top app bar height results in ' + 'currentAppBarOffsetTop_ being negative topAppBarHeight_', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(100); - foundation.handleScroll(); + foundation.handleTargetScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === -64); }); -test('top app bar : handleScroll scrolling does not generate a ' + +test('top app bar : handleTargetScroll scrolling does not generate a ' + 'positive currentAppBarOffsetTop_', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getViewportScrollY()).thenReturn(100); - foundation.handleScroll(); + foundation.handleTargetScroll(); td.when(mockAdapter.getViewportScrollY()).thenReturn(-100); - foundation.handleScroll(); + foundation.handleTargetScroll(); assert.isTrue(foundation.currentAppBarOffsetTop_ === 0); }); test('top app bar : resize events should set isCurrentlyBeingResized_ to true', () => { const {foundation} = setupTest(); - foundation.handleResize(); + foundation.handleWindowResize(); assert.isTrue(foundation.isCurrentlyBeingResized_); }); @@ -172,9 +172,9 @@ test('top app bar : resize events throttle multiple calls of throttledResizeHand const clock = installClock(); const {foundation} = setupTest(); - foundation.handleResize(); + foundation.handleWindowResize(); assert.isFalse(!foundation.resizeThrottleId_); - foundation.handleResize(); + foundation.handleWindowResize(); clock.tick(numbers.DEBOUNCE_THROTTLE_RESIZE_TIME_MS); assert.isTrue(!foundation.resizeThrottleId_); }); @@ -183,10 +183,10 @@ test('top app bar : resize events debounce changing isCurrentlyBeingResized_ to const clock = installClock(); const {foundation} = setupTest(); - foundation.handleResize(); + foundation.handleWindowResize(); const debounceId = foundation.resizeDebounceId_; clock.tick(50); - foundation.handleResize(); + foundation.handleWindowResize(); assert.isFalse(debounceId === foundation.resizeDebounceId_); assert.isTrue(foundation.isCurrentlyBeingResized_); clock.tick(150); From 20a6ce0c17ac59ee6059ac1ad175e2e402cff9a9 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Tue, 14 May 2019 12:37:00 -0700 Subject: [PATCH 12/17] fix: eventlistener logic --- packages/mdc-top-app-bar/component.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/mdc-top-app-bar/component.ts b/packages/mdc-top-app-bar/component.ts index 8a38d9487b6..a6c419e5c47 100644 --- a/packages/mdc-top-app-bar/component.ts +++ b/packages/mdc-top-app-bar/component.ts @@ -62,19 +62,22 @@ export class MDCTopAppBar extends MDCComponent { } initialSyncWithDOM() { + const isFixed = this.root_.classList.contains(cssClasses.FIXED_CLASS); + const isShort = this.root_.classList.contains(cssClasses.SHORT_CLASS); + this.handleNavigationClick_ = this.foundation_.handleNavigationClick.bind(this.foundation_); this.handleWindowResize_ = this.foundation_.handleWindowResize && this.foundation_.handleWindowResize.bind(this.foundation_); this.handleTargetScroll_ = this.foundation_.handleTargetScroll && this.foundation_.handleTargetScroll.bind(this.foundation_); + this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); + if (this.navIcon_) { this.navIcon_.addEventListener('click', this.handleNavigationClick_ as EventListener); } - if (this.handleTargetScroll_) { - this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); - } - if (this.handleWindowResize_) { + + if (!isShort && !isFixed) { window.addEventListener('resize', this.handleWindowResize_ as EventListener); } } From 799be223a1c9b7caee26af9cee26c1b3f79ca24b Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Wed, 15 May 2019 14:00:34 -0700 Subject: [PATCH 13/17] fix: pr updatesfix: pr updatesfix: pr updates --- packages/mdc-top-app-bar/README.md | 2 +- packages/mdc-top-app-bar/component.ts | 23 +++++++++----------- packages/mdc-top-app-bar/foundation.ts | 6 ++--- packages/mdc-top-app-bar/short/foundation.ts | 4 +--- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/mdc-top-app-bar/README.md b/packages/mdc-top-app-bar/README.md index 35c56bbd961..9f07d8d8d4e 100644 --- a/packages/mdc-top-app-bar/README.md +++ b/packages/mdc-top-app-bar/README.md @@ -228,7 +228,7 @@ All foundations provide the following methods: Method Signature | Description --- | --- -`handleTargetScroll() => void` | Handles `scroll` event on specified scrollTarget (default to `window`). +`handleTargetScroll() => void` | Handles `scroll` event on specified scrollTarget (defaults to `window`). `handleWindowResize() => void` | Handles `resize` event on window. `handleNavigationClick() => void` | Handles `click` event on navigation icon. diff --git a/packages/mdc-top-app-bar/component.ts b/packages/mdc-top-app-bar/component.ts index a6c419e5c47..f3fe148b08d 100644 --- a/packages/mdc-top-app-bar/component.ts +++ b/packages/mdc-top-app-bar/component.ts @@ -37,8 +37,8 @@ export class MDCTopAppBar extends MDCComponent { } private handleNavigationClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() - private handleWindowResize_?: SpecificEventListener<'resize'>; // assigned in initialSyncWithDOM() - private handleTargetScroll_?: SpecificEventListener<'scroll'>; // assigned in initialSyncWithDOM() + private handleWindowResize_!: SpecificEventListener<'resize'>; // assigned in initialSyncWithDOM() + private handleTargetScroll_!: SpecificEventListener<'scroll'>; // assigned in initialSyncWithDOM() private navIcon_!: Element | null; private iconRipples_!: MDCRipple[]; private scrollTarget_!: EventTarget; @@ -62,14 +62,9 @@ export class MDCTopAppBar extends MDCComponent { } initialSyncWithDOM() { - const isFixed = this.root_.classList.contains(cssClasses.FIXED_CLASS); - const isShort = this.root_.classList.contains(cssClasses.SHORT_CLASS); - this.handleNavigationClick_ = this.foundation_.handleNavigationClick.bind(this.foundation_); - this.handleWindowResize_ - = this.foundation_.handleWindowResize && this.foundation_.handleWindowResize.bind(this.foundation_); - this.handleTargetScroll_ - = this.foundation_.handleTargetScroll && this.foundation_.handleTargetScroll.bind(this.foundation_); + this.handleWindowResize_ = this.foundation_.handleWindowResize.bind(this.foundation_); + this.handleTargetScroll_ = this.foundation_.handleTargetScroll.bind(this.foundation_); this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); @@ -77,6 +72,8 @@ export class MDCTopAppBar extends MDCComponent { this.navIcon_.addEventListener('click', this.handleNavigationClick_ as EventListener); } + const isFixed = this.root_.classList.contains(cssClasses.FIXED_CLASS); + const isShort = this.root_.classList.contains(cssClasses.SHORT_CLASS); if (!isShort && !isFixed) { window.addEventListener('resize', this.handleWindowResize_ as EventListener); } @@ -84,13 +81,13 @@ export class MDCTopAppBar extends MDCComponent { destroy() { this.iconRipples_.forEach((iconRipple) => iconRipple.destroy()); + this.scrollTarget_.removeEventListener('scroll', this.handleTargetScroll_ as EventListener); if (this.navIcon_) { this.navIcon_.removeEventListener('click', this.handleNavigationClick_ as EventListener); } - if (this.handleTargetScroll_) { - this.scrollTarget_.removeEventListener('scroll', this.handleTargetScroll_ as EventListener); - } - if (this.handleWindowResize_) { + const isFixed = this.root_.classList.contains(cssClasses.FIXED_CLASS); + const isShort = this.root_.classList.contains(cssClasses.SHORT_CLASS); + if (!isShort && !isFixed) { window.removeEventListener('resize', this.handleWindowResize_ as EventListener); } super.destroy(); diff --git a/packages/mdc-top-app-bar/foundation.ts b/packages/mdc-top-app-bar/foundation.ts index 65d752bf511..4de590b9a64 100644 --- a/packages/mdc-top-app-bar/foundation.ts +++ b/packages/mdc-top-app-bar/foundation.ts @@ -57,14 +57,14 @@ export class MDCTopAppBarBaseFoundation extends MDCFoundation; - handleWindowResize?: SpecificEventListener<'resize'>; - /* istanbul ignore next: optional argument is not a branch statement */ constructor(adapter?: Partial) { super({...MDCTopAppBarBaseFoundation.defaultAdapter, ...adapter}); } + handleTargetScroll: SpecificEventListener<'scroll'> = () => undefined; + handleWindowResize: SpecificEventListener<'resize'> = () => undefined; + handleNavigationClick() { this.adapter_.notifyNavigationIconClicked(); } diff --git a/packages/mdc-top-app-bar/short/foundation.ts b/packages/mdc-top-app-bar/short/foundation.ts index e886433ecc4..d6325321208 100644 --- a/packages/mdc-top-app-bar/short/foundation.ts +++ b/packages/mdc-top-app-bar/short/foundation.ts @@ -45,9 +45,7 @@ export class MDCShortTopAppBarFoundation extends MDCTopAppBarBaseFoundation { this.adapter_.addClass(cssClasses.SHORT_HAS_ACTION_ITEM_CLASS); } - if (!this.adapter_.hasClass(cssClasses.SHORT_COLLAPSED_CLASS)) { - this.handleTargetScroll(); - } + this.handleTargetScroll(); } /** From a9e956671d3ff44f48022703967e918a87444afe Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Wed, 15 May 2019 15:47:21 -0700 Subject: [PATCH 14/17] fix: tests and foundation code --- packages/mdc-top-app-bar/fixed/foundation.ts | 2 +- packages/mdc-top-app-bar/foundation.ts | 5 ++--- packages/mdc-top-app-bar/short/foundation.ts | 2 +- packages/mdc-top-app-bar/standard/foundation.ts | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/mdc-top-app-bar/fixed/foundation.ts b/packages/mdc-top-app-bar/fixed/foundation.ts index 76f96a98a10..584138a944f 100644 --- a/packages/mdc-top-app-bar/fixed/foundation.ts +++ b/packages/mdc-top-app-bar/fixed/foundation.ts @@ -33,7 +33,7 @@ export class MDCFixedTopAppBarFoundation extends MDCTopAppBarFoundation { /** * Scroll handler for applying/removing the modifier class on the fixed top app bar. */ - handleTargetScroll = () => { + handleTargetScroll() { const currentScroll = this.adapter_.getViewportScrollY(); if (currentScroll <= 0) { diff --git a/packages/mdc-top-app-bar/foundation.ts b/packages/mdc-top-app-bar/foundation.ts index 4de590b9a64..423db094cfe 100644 --- a/packages/mdc-top-app-bar/foundation.ts +++ b/packages/mdc-top-app-bar/foundation.ts @@ -22,7 +22,6 @@ */ import {MDCFoundation} from '@material/base/foundation'; -import {SpecificEventListener} from '@material/base/types'; import {MDCTopAppBarAdapter} from './adapter'; import {cssClasses, numbers, strings} from './constants'; @@ -62,8 +61,8 @@ export class MDCTopAppBarBaseFoundation extends MDCFoundation = () => undefined; - handleWindowResize: SpecificEventListener<'resize'> = () => undefined; + handleTargetScroll() {} // tslint:disable-line:no-empty + handleWindowResize() {} // tslint:disable-line:no-empty handleNavigationClick() { this.adapter_.notifyNavigationIconClicked(); diff --git a/packages/mdc-top-app-bar/short/foundation.ts b/packages/mdc-top-app-bar/short/foundation.ts index d6325321208..6d87689d9e0 100644 --- a/packages/mdc-top-app-bar/short/foundation.ts +++ b/packages/mdc-top-app-bar/short/foundation.ts @@ -51,7 +51,7 @@ export class MDCShortTopAppBarFoundation extends MDCTopAppBarBaseFoundation { /** * Scroll handler for applying/removing the collapsed modifier class on the short top app bar. */ - handleTargetScroll = () => { + handleTargetScroll() { if (this.adapter_.hasClass(cssClasses.SHORT_COLLAPSED_CLASS)) { return; } diff --git a/packages/mdc-top-app-bar/standard/foundation.ts b/packages/mdc-top-app-bar/standard/foundation.ts index 9e8b0941649..dcd4cf21fef 100644 --- a/packages/mdc-top-app-bar/standard/foundation.ts +++ b/packages/mdc-top-app-bar/standard/foundation.ts @@ -84,7 +84,7 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { /** * Scroll handler for the default scroll behavior of the top app bar. */ - handleTargetScroll = () => { + handleTargetScroll() { const currentScrollPosition = Math.max(this.adapter_.getViewportScrollY(), 0); const diff = currentScrollPosition - this.lastScrollPosition_; this.lastScrollPosition_ = currentScrollPosition; @@ -107,7 +107,7 @@ export class MDCTopAppBarFoundation extends MDCTopAppBarBaseFoundation { /** * Top app bar resize handler that throttle/debounce functions that execute updates. */ - handleWindowResize = () => { + handleWindowResize() { // Throttle resize events 10 p/s if (!this.resizeThrottleId_) { this.resizeThrottleId_ = setTimeout(() => { From 7de95ed364790625af581823050fa86bec3129ee Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Wed, 15 May 2019 16:47:47 -0700 Subject: [PATCH 15/17] fix: add test --- packages/mdc-top-app-bar/short/foundation.ts | 3 +++ test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/packages/mdc-top-app-bar/short/foundation.ts b/packages/mdc-top-app-bar/short/foundation.ts index 6d87689d9e0..97367dee256 100644 --- a/packages/mdc-top-app-bar/short/foundation.ts +++ b/packages/mdc-top-app-bar/short/foundation.ts @@ -45,6 +45,9 @@ export class MDCShortTopAppBarFoundation extends MDCTopAppBarBaseFoundation { this.adapter_.addClass(cssClasses.SHORT_HAS_ACTION_ITEM_CLASS); } + // this was a private method call before, but we decided to override the base class method instead. + // this is intended as the short variant must calculate if the + // page starts off from the top of the page. this.handleTargetScroll(); } diff --git a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js index 1bf2f3aa095..ae18b43c422 100644 --- a/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js +++ b/test/unit/mdc-top-app-bar/mdc-top-app-bar.test.js @@ -256,3 +256,11 @@ test('adapter#getTotalActionItems returns the number of action items on the oppo const actual = root.querySelectorAll(strings.ACTION_ITEM_SELECTOR).length; assert.isTrue(adapterReturn === actual); }); + +test('adapter#notifyNavigationIconClicked emits the NAVIGATION_EVENT', () => { + const {component} = setupTest(); + const callback = td.func(); + component.listen(strings.NAVIGATION_EVENT, callback); + component.getDefaultFoundation().adapter_.notifyNavigationIconClicked(); + td.verify(callback(td.matchers.isA(Object)), {times: 1}); +}); From 12608dfa758f29fe3378be4d37d0290a76577703 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Thu, 16 May 2019 10:41:17 -0700 Subject: [PATCH 16/17] fix: add comments --- packages/mdc-top-app-bar/component.ts | 12 ++++-------- packages/mdc-top-app-bar/fixed/foundation.ts | 1 + packages/mdc-top-app-bar/foundation.ts | 2 ++ packages/mdc-top-app-bar/short/foundation.ts | 2 +- packages/mdc-top-app-bar/standard/foundation.ts | 2 ++ 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/mdc-top-app-bar/component.ts b/packages/mdc-top-app-bar/component.ts index f3fe148b08d..c28805c701f 100644 --- a/packages/mdc-top-app-bar/component.ts +++ b/packages/mdc-top-app-bar/component.ts @@ -95,18 +95,14 @@ export class MDCTopAppBar extends MDCComponent { setScrollTarget(target: EventTarget) { // Remove scroll handler from the previous scroll target - if (this.handleTargetScroll_) { - this.scrollTarget_.removeEventListener('scroll', this.handleTargetScroll_ as EventListener); - } + this.scrollTarget_.removeEventListener('scroll', this.handleTargetScroll_ as EventListener); this.scrollTarget_ = target; // Initialize scroll handler on the new scroll target - if (this.handleTargetScroll_) { - this.handleTargetScroll_ - = this.foundation_.handleTargetScroll && this.foundation_.handleTargetScroll.bind(this.foundation_); - this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); - } + this.handleTargetScroll_ = + this.foundation_.handleTargetScroll.bind(this.foundation_); + this.scrollTarget_.addEventListener('scroll', this.handleTargetScroll_ as EventListener); } getDefaultFoundation() { diff --git a/packages/mdc-top-app-bar/fixed/foundation.ts b/packages/mdc-top-app-bar/fixed/foundation.ts index 584138a944f..b8f87d28062 100644 --- a/packages/mdc-top-app-bar/fixed/foundation.ts +++ b/packages/mdc-top-app-bar/fixed/foundation.ts @@ -32,6 +32,7 @@ export class MDCFixedTopAppBarFoundation extends MDCTopAppBarFoundation { /** * Scroll handler for applying/removing the modifier class on the fixed top app bar. + * @override */ handleTargetScroll() { const currentScroll = this.adapter_.getViewportScrollY(); diff --git a/packages/mdc-top-app-bar/foundation.ts b/packages/mdc-top-app-bar/foundation.ts index 423db094cfe..5392ac1fffd 100644 --- a/packages/mdc-top-app-bar/foundation.ts +++ b/packages/mdc-top-app-bar/foundation.ts @@ -61,7 +61,9 @@ export class MDCTopAppBarBaseFoundation extends MDCFoundation Date: Thu, 16 May 2019 12:13:16 -0700 Subject: [PATCH 17/17] fix: increase test cov --- test/unit/mdc-top-app-bar/fixed.foundation.test.js | 8 ++++++++ test/unit/mdc-top-app-bar/short.foundation.test.js | 14 ++++++++++++++ .../mdc-top-app-bar/standard.foundation.test.js | 6 ++++++ 3 files changed, 28 insertions(+) diff --git a/test/unit/mdc-top-app-bar/fixed.foundation.test.js b/test/unit/mdc-top-app-bar/fixed.foundation.test.js index 60cda84ed52..fc2e1d0a302 100644 --- a/test/unit/mdc-top-app-bar/fixed.foundation.test.js +++ b/test/unit/mdc-top-app-bar/fixed.foundation.test.js @@ -66,3 +66,11 @@ test('#handleTargetScroll does not call #adapter.removeClass if was not scrolled foundation.handleTargetScroll(); td.verify(mockAdapter.removeClass(td.matchers.isA(String)), {times: 0}); }); + +test('#handleTargetScroll calls #adapter.addClass only once if it already scrolled', () => { + const {foundation, mockAdapter} = setupTest(); + td.when(mockAdapter.getViewportScrollY()).thenReturn(1); + foundation.handleTargetScroll(); + foundation.handleTargetScroll(); + td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.FIXED_SCROLLED_CLASS), {times: 1}); +}); diff --git a/test/unit/mdc-top-app-bar/short.foundation.test.js b/test/unit/mdc-top-app-bar/short.foundation.test.js index 8dc5a86d5e1..33111261a70 100644 --- a/test/unit/mdc-top-app-bar/short.foundation.test.js +++ b/test/unit/mdc-top-app-bar/short.foundation.test.js @@ -21,6 +21,7 @@ * THE SOFTWARE. */ +import {assert} from 'chai'; import td from 'testdouble'; import {MDCShortTopAppBarFoundation} from '../../../packages/mdc-top-app-bar/short/foundation'; @@ -96,3 +97,16 @@ test('short top app bar: #adapter.addClass is not called if it does not have an foundation.init(); td.verify(mockAdapter.addClass(MDCTopAppBarFoundation.cssClasses.SHORT_HAS_ACTION_ITEM_CLASS), {times: 0}); }); + +test('isCollapsed returns false initially if top-app-bar has not scrolled', () => { + const {foundation} = setupTest(); + assert.isFalse(foundation.isCollapsed); +}); + +test('isCollapsed returns true if top-app-bar is collapsed', () => { + const {foundation, mockAdapter} = setupTest(); + td.when(mockAdapter.getViewportScrollY()).thenReturn(1); + + foundation.handleTargetScroll(); + assert.isTrue(foundation.isCollapsed); +}); diff --git a/test/unit/mdc-top-app-bar/standard.foundation.test.js b/test/unit/mdc-top-app-bar/standard.foundation.test.js index 206ab763d9b..745e8604de2 100644 --- a/test/unit/mdc-top-app-bar/standard.foundation.test.js +++ b/test/unit/mdc-top-app-bar/standard.foundation.test.js @@ -192,3 +192,9 @@ test('top app bar : resize events debounce changing isCurrentlyBeingResized_ to clock.tick(150); assert.isFalse(foundation.isCurrentlyBeingResized_); }); + +test('#destroy calls #adapter.setStyle(top, "")', () => { + const {foundation, mockAdapter} = setupTest(); + foundation.destroy(); + td.verify(mockAdapter.setStyle('top', ''), {times: 1}); +});