Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

feat(dialog): Reverse buttons when stacked; allow toggling auto-stack #3573

Merged
merged 4 commits into from
Sep 19, 2018
+162 −36
Merged

feat(dialog): Reverse buttons when stacked; allow toggling auto-stack #3573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
105ed9a
feat(dialog): Reverse buttons when stacked; allow toggling auto-stack
Sep 17, 2018
052723c
WIP Update goldens
Sep 17, 2018
da94125
WIP move --stacked detect to foundation, update tests
Sep 18, 2018
284e16c
Merge branch 'master' into feat/dialog-stacked-reverse
Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions packages/mdc-dialog/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,25 @@ Any action buttons within the dialog which equate strictly to a dismissal with n
`close` action; this will make it easy to handle all such interactions consistently, while separately handling other
actions.

#### Action Button Arrangement

As indicated in the [Dialog design article](https://material.io/design/components/dialogs.html#anatomy), buttons within
the `mdc-dialog__actions` element are arranged horizontally by default, with the confirming action _last_.

In cases where the button text is too long for all buttons to fit on a single line, the buttons are stacked vertically,
with the confirming action _first_.

MDC Dialog detects and handles this automatically by default, reversing the buttons when applying the stacked layout.
This automatic behavior can be disabled by setting `autoStackButtons` to `false` on the component instance:

```js
dialog.autoStackButtons = false;
```

This will also be disabled if the `mdc-dialog--stacked` modifier class is applied manually to the root element before the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this implemented in foundation? couldn't find logic which detects the existence of --stacked modifier class before auto stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's currently implemented in the component (where you left another comment), but I'm moving it to the foundation.

component is instantiated, but note that dialog action button labels are recommended to be short enough to fit on a
single line if possible.

#### Actions and Selections

Dialogs which require making a choice via selection controls should initially disable any button which performs an
Expand Down Expand Up @@ -243,6 +262,7 @@ Property | Value Type | Description
`isOpen` | `boolean` (read-only) | Proxies to the foundation's `isOpen` method.
`escapeKeyAction` | `string` | Proxies to the foundation's `getEscapeKeyAction` and `setEscapeKeyAction` methods.
`scrimClickAction` | `string` | Proxies to the foundation's `getScrimClickAction` and `setScrimClickAction` methods.
`autoStackButtons` | `boolean` | Proxies to the foundation's `getAutoStackButtons` and `setAutoStackButtons` methods.

Method Signature | Description
--- | ---
Expand All @@ -269,6 +289,7 @@ Method Signature | Description
--- | ---
`addClass(className: string) => void` | Adds a class to the root element.
`removeClass(className: string) => void` | Removes a class from the root element.
`hasClass(className: string) => boolean` | Returns whether the given class exists on the root element.
`addBodyClass(className: string) => void` | Adds a class to the `<body>`.
`removeBodyClass(className: string) => void` | Removes a class from the `<body>`.
`eventTargetHasClass(target: !EventTarget, className: string) => void` | Returns `true` if the target element has the given CSS class, otherwise `false`.
Expand All @@ -278,6 +299,7 @@ Method Signature | Description
`isContentScrollable() => boolean` | Returns `true` if `mdc-dialog__content` can be scrolled by the user, otherwise `false`.
`areButtonsStacked() => boolean` | Returns `true` if `mdc-dialog__action` buttons (`mdc-dialog__button`) are stacked vertically, otherwise `false` if they are side-by-side.
`getActionFromEvent(event: !Event) => ?string` | Retrieves the value of the `data-mdc-dialog-action` attribute from the given event's target, or an ancestor of the target.
`reverseButtons() => void` | Reverses the order of action buttons in the `mdc-dialog__actions` element. Used when switching between stacked and unstacked button layouts.
`notifyOpening() => void` | Broadcasts an event denoting that the dialog has just started to open.
`notifyOpened() => void` | Broadcasts an event denoting that the dialog has finished opening.
`notifyClosing(action: string) {}` | Broadcasts an event denoting that the dialog has just started closing. If a non-empty `action` is passed, the event's `detail` object should include its value in the `action` property.
Expand All @@ -295,6 +317,8 @@ Method Signature | Description
`setEscapeKeyAction(action: string)` | Sets the action reflected when the Escape key is pressed. Setting to `''` disables closing the dialog via Escape key.
`getScrimClickAction() => string` | Returns the action reflected when the scrim is clicked.
`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.
`handleClick(event: Event)` | Handles `click` 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

Expand Down
8 changes: 8 additions & 0 deletions packages/mdc-dialog/adapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ class MDCDialogAdapter {
/** @param {string} className */
removeClass(className) {}

/**
* @param {string} className
* @return {boolean}
*/
hasClass(className) {}

/** @param {string} className */
addBodyClass(className) {}

Expand Down Expand Up @@ -77,6 +83,8 @@ class MDCDialogAdapter {
*/
getActionFromEvent(event) {}

reverseButtons() {}

notifyOpening() {}
notifyOpened() {}

Expand Down
38 changes: 36 additions & 2 deletions packages/mdc-dialog/foundation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class MDCDialogFoundation extends MDCFoundation {
return /** @type {!MDCDialogAdapter} */ ({
addClass: (/* className: string */) => {},
removeClass: (/* className: string */) => {},
hasClass: (/* className: string */) => {},
addBodyClass: (/* className: string */) => {},
removeBodyClass: (/* className: string */) => {},
eventTargetHasClass: (/* target: !EventTarget, className: string */) => {},
Expand All @@ -51,6 +52,7 @@ class MDCDialogFoundation extends MDCFoundation {
isContentScrollable: () => {},
areButtonsStacked: () => {},
getActionFromEvent: (/* event: !Event */) => {},
reverseButtons: () => {},
notifyOpening: () => {},
notifyOpened: () => {},
notifyClosing: (/* action: ?string */) => {},
Expand Down Expand Up @@ -78,6 +80,18 @@ class MDCDialogFoundation extends MDCFoundation {

/** @private {string} */
this.scrimClickAction_ = strings.CLOSE_ACTION;

/** @private {boolean} */
this.autoStackButtons_ = true;

/** @private {boolean} */
this.areButtonsStacked_ = false;
};

init() {
if (this.adapter_.hasClass(cssClasses.STACKED)) {
this.setAutoStackButtons(false);
}
};

destroy() {
Expand Down Expand Up @@ -158,6 +172,16 @@ class MDCDialogFoundation extends MDCFoundation {
this.scrimClickAction_ = action;
}

/** @return {boolean} */
getAutoStackButtons() {
return this.autoStackButtons_;
}

/** @param {boolean} autoStack */
setAutoStackButtons(autoStack) {
this.autoStackButtons_ = autoStack;
}

layout() {
if (this.layoutFrame_) {
cancelAnimationFrame(this.layoutFrame_);
Expand All @@ -169,17 +193,27 @@ class MDCDialogFoundation extends MDCFoundation {
}

layoutInternal_() {
this.detectStackedButtons_();
if (this.autoStackButtons_) {
this.detectStackedButtons_();
}
this.detectScrollableContent_();
}

/** @private */
detectStackedButtons_() {
// Remove the class first to let us measure the buttons' natural positions.
this.adapter_.removeClass(cssClasses.STACKED);
if (this.adapter_.areButtonsStacked()) {

const areButtonsStacked = this.adapter_.areButtonsStacked();

if (areButtonsStacked) {
this.adapter_.addClass(cssClasses.STACKED);
}

if (areButtonsStacked !== this.areButtonsStacked_) {
this.adapter_.reverseButtons();
this.areButtonsStacked_ = areButtonsStacked;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment can be outside if condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's outside the if, it's going to assign the value even when it's already the same value. Putting it inside the if makes it only assign the value when it's actually going to do something.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I think we don't have to condition this since it is not toggling as in .reverseButton(). But this is fine too.

}
}

/** @private */
Expand Down
13 changes: 13 additions & 0 deletions packages/mdc-dialog/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ class MDCDialog extends MDCComponent {
this.foundation_.setScrimClickAction(action);
}

get autoStackButtons() {
return this.foundation_.getAutoStackButtons();
}

set autoStackButtons(autoStack) {
this.foundation_.setAutoStackButtons(autoStack);
}

initialize(focusTrapFactory = createFocusTrap, initialFocusEl = null) {
this.container_ = /** @type {!Element} */ (this.root_.querySelector(strings.CONTAINER_SELECTOR));
this.content_ = this.root_.querySelector(strings.CONTENT_SELECTOR);
Expand Down Expand Up @@ -164,6 +172,7 @@ class MDCDialog extends MDCComponent {
return new MDCDialogFoundation({
addClass: (className) => this.root_.classList.add(className),
removeClass: (className) => this.root_.classList.remove(className),
hasClass: (className) => this.root_.classList.contains(className),
addBodyClass: (className) => document.body.classList.add(className),
removeBodyClass: (className) => document.body.classList.remove(className),
eventTargetHasClass: (target, className) => target.classList.contains(className),
Expand All @@ -176,6 +185,10 @@ class MDCDialog extends MDCComponent {
const element = closest(event.target, `[${strings.ACTION_ATTRIBUTE}]`);
return element && element.getAttribute(strings.ACTION_ATTRIBUTE);
},
reverseButtons: () => {
this.buttons_.reverse();
this.buttons_.forEach((button) => button.parentElement.appendChild(button));
},
notifyOpening: () => this.emit(strings.OPENING_EVENT, {}),
notifyOpened: () => this.emit(strings.OPENED_EVENT, {}),
notifyClosing: (action) => this.emit(strings.CLOSING_EVENT, action ? {action} : {}),
Expand Down
40 changes: 20 additions & 20 deletions test/screenshot/golden.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,27 +405,27 @@
}
},
"spec/mdc-dialog/classes/overflow-accessible-font-size.html": {
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-accessible-font-size.html?utm_source=golden_json",
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-accessible-font-size.html?utm_source=golden_json",
"screenshots": {
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-accessible-font-size.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-accessible-font-size.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-accessible-font-size.html.windows_firefox_62.png"
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-accessible-font-size.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-accessible-font-size.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-accessible-font-size.html.windows_firefox_62.png"
}
},
"spec/mdc-dialog/classes/overflow-bottom.html": {
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/13/04_52_25_966/spec/mdc-dialog/classes/overflow-bottom.html?utm_source=golden_json",
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-bottom.html?utm_source=golden_json",
"screenshots": {
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-bottom.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-bottom.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-bottom.html.windows_firefox_62.png"
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-bottom.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-bottom.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-bottom.html.windows_firefox_62.png"
}
},
"spec/mdc-dialog/classes/overflow-top.html": {
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-top.html?utm_source=golden_json",
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-top.html?utm_source=golden_json",
"screenshots": {
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-top.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-top.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/classes/overflow-top.html.windows_firefox_62.png"
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-top.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-top.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-top.html.windows_firefox_62.png"
}
},
"spec/mdc-dialog/mixins/container-fill-color.html": {
Expand Down Expand Up @@ -453,19 +453,19 @@
}
},
"spec/mdc-dialog/mixins/max-height.html": {
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-height.html?utm_source=golden_json",
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-height.html?utm_source=golden_json",
"screenshots": {
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-height.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-height.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-height.html.windows_firefox_62.png"
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-height.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-height.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-height.html.windows_firefox_62.png"
}
},
"spec/mdc-dialog/mixins/max-width.html": {
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-width.html?utm_source=golden_json",
"public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-width.html?utm_source=golden_json",
"screenshots": {
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-width.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-width.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/12/22_19_10_073/spec/mdc-dialog/mixins/max-width.html.windows_firefox_62.png"
"desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-width.html.windows_chrome_69.png",
"desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-width.html.windows_edge_17.png",
"desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/mixins/max-width.html.windows_firefox_62.png"
}
},
"spec/mdc-dialog/mixins/min-width.html": {
Expand Down
Loading