Skip to content

Commit

Permalink
Improve ContentDialog's response to the InputPane
Browse files Browse the repository at this point in the history 
Fixes #1149 and #910

When the input pane shows, the ContentDialog's response was to maintain
its top/left position and to shrink its height as much as needed to avoid
being occluded by the input pane. This behavior resulted in a bug where,
if the dialog's top/left was close to the center of the screen, the dialog
could become very short when it resized to get out of the way of the input
pane (#1149).

To fix this, the new way the ContentDialog responds to the input pane is:
  - If the dialog will not be occluded by the input pane, then the dialog
    maintains its current size and position.
  - If the dialog will be occluded by the input pane, the dialog lays
    itself out using its normal layout rules but it does so in the
    unoccluded portion of the window (visible document) rather than in the
    whole window.

There was another bug (#910) where the ContentDialog would become
horizontally off center if the window was resized while the input pane was
showing. This is now fixed because the ContentDialog uses its normal
layout rules even when the input pane is showing.

Other changes:
  - Switiched from .focus() to .scrollIntoView() for keeping the focused
    element in view while resizing the dialog to get out of the input
    pane's way. The motivation for this change was that calling .focus()
    on document.activeElement no longer causes the element to scroll into
    view in Edge.
  - When the ContentDialog is shown, put focus on the root element of the
    dialog rather than on the first focusable element within the dialog's
    content area. The motivation for this change is that .scrollIntoView()
    is more aggressive about scrolling than .focus() was -- it scrolls the
    element to the top of its scroller even if that element was already in
    view. This results in a poor experience when showing the dialog and
    the first focusable element in the content area is a textfield -- the
    user will never have the chance to see the content at the top of the
    dialog because the dialog would immediately scroll it off screen.
    Order of events:
      - Dialog shows and puts focus on the first focusable element
        (assume it's a textfield) in the content area
      - Input pane shows
      - Dialog resizes
      - Dialog calls .scrollIntView() on the textfield to keep it in view
	during the resize thus scrolling the dialog's topmost content off
	the screen
  • Loading branch information
Adam Comella committed Aug 15, 2015
1 parent 30eabf1 commit f4a93a5
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 64 deletions.
167 changes: 114 additions & 53 deletions src/js/WinJS/Controls/ContentDialog.js
View file
Expand Up @@ -15,11 +15,12 @@ define([
'../Core/_Resources',
'../Utilities/_Control',
'../Utilities/_ElementUtilities',
'../Utilities/_KeyboardInfo',
'../Utilities/_Hoverable',
'../Animations',
'require-style!less/styles-contentdialog',
'require-style!less/colors-contentdialog'
], function contentDialogInit(Application, _Dispose, _Accents, Promise, _Signal, _LightDismissService, _BaseUtils, _Global, _WinRT, _Base, _Events, _ErrorFromName, _Resources, _Control, _ElementUtilities, _Hoverable, _Animations) {
], function contentDialogInit(Application, _Dispose, _Accents, Promise, _Signal, _LightDismissService, _BaseUtils, _Global, _WinRT, _Base, _Events, _ErrorFromName, _Resources, _Control, _ElementUtilities, _KeyboardInfo, _Hoverable, _Animations) {
"use strict";

_Accents.createAccentRule(".win-contentdialog-dialog", [{ name: "outline-color", value: _Accents.ColorTypes.accent }]);
Expand Down Expand Up @@ -83,15 +84,25 @@ define([
_column0or1: "win-contentdialog-column0or1",
_visible: "win-contentdialog-visible",
_tabStop: "win-contentdialog-tabstop",
_commandSpacer: "win-contentdialog-commandspacer"
_commandSpacer: "win-contentdialog-commandspacer",
_deviceFixedSupported: "win-contentdialog-devicefixedsupported"
};
var EventNames = {
beforeShow: "beforeshow",
afterShow: "aftershow",
beforeHide: "beforehide",
afterHide: "afterhide",
};
var minContentHeightWithInputPane = 96;

var _supportsMsDeviceFixedCached;
function supportsMsDevicedFixed(dialogRoot) {
if (typeof _supportsMsDeviceFixedCached === "undefined") {
var element = _Global.document.createElement("div");
element.style.position = "-ms-device-fixed";
_supportsMsDeviceFixedCached = (_ElementUtilities._getComputedStyle(element).position === "-ms-device-fixed");
}
return _supportsMsDeviceFixedCached;
}

// WinJS animation promises always complete successfully. This
// helper allows an animation promise to complete in the canceled state
Expand Down Expand Up @@ -206,7 +217,6 @@ define([
},
onTakeFocus: function (useSetActive) {
dialog._dismissable.restoreFocus() ||
_ElementUtilities._focusFirstFocusableElement(dialog._dom.content) ||
_ElementUtilities._tryFocusOnAnyElement(dialog._dom.dialog, useSetActive);
}
});
Expand Down Expand Up @@ -287,11 +297,8 @@ define([
that._pendingHide = null;
_ElementUtilities.addClass(that.dialog._dom.root, ClassNames._visible);
that.dialog._addExternalListeners();
if (_WinRT.Windows.UI.ViewManagement.InputPane) {
var inputPaneHeight = _WinRT.Windows.UI.ViewManagement.InputPane.getForCurrentView().occludedRect.height;
if (inputPaneHeight > 0) {
that.dialog._renderForInputPane(inputPaneHeight);
}
if (_KeyboardInfo._KeyboardInfo._visible) {
that.dialog._renderForInputPane();
}
_LightDismissService.shown(that.dialog._dismissable);
return that.dialog._playEntranceAnimation();
Expand Down Expand Up @@ -461,10 +468,16 @@ define([

this._onInputPaneShownBound = this._onInputPaneShown.bind(this);
this._onInputPaneHiddenBound = this._onInputPaneHidden.bind(this);
this._onUpdateInputPaneRenderingBound = this._onUpdateInputPaneRendering.bind(this);

this._disposed = false;
this._resizedForInputPane = false;
this._currentFocus = null;
this._rendered = {
registeredForResize: false,
resizedForInputPane: false,
top: "",
bottom: ""
};

this._initializeDom(element || _Global.document.createElement("div"));
this._setState(States.Init);
Expand Down Expand Up @@ -601,6 +614,7 @@ define([
}
this._setState(States.Disposed);
this._disposed = true;
_ElementUtilities._resizeNotifier.unsubscribe(this._dom.root, this._onUpdateInputPaneRenderingBound);
_Dispose._disposeElement(this._dom.content);
},

Expand Down Expand Up @@ -696,6 +710,10 @@ define([
_ElementUtilities._addEventListener(dom.endBodyTab, "focusin", this._onEndBodyTabFocusIn.bind(this));
dom.commands[0].addEventListener("click", this._onCommandClicked.bind(this, DismissalResult.primary));
dom.commands[1].addEventListener("click", this._onCommandClicked.bind(this, DismissalResult.secondary));

if (supportsMsDevicedFixed()) {
_ElementUtilities.addClass(dom.root, ClassNames._deviceFixedSupported);
}
},

_updateCommandsUI: function ContentDialog_updateCommandsUI() {
Expand Down Expand Up @@ -780,6 +798,13 @@ define([
_onInputPaneHidden: function ContentDialog_onInputPaneHidden() {
this._state.onInputPaneHidden();
},

_onUpdateInputPaneRendering: function ContentDialog_onUpdateInputPaneRendering() {
// When the dialog and the input pane are shown and the window resizes, this may
// change the visual document's dimensions so we may need to update the rendering
// of the dialog.
this._renderForInputPane();
},

//
// Methods called by states
Expand Down Expand Up @@ -853,61 +878,97 @@ define([

_addExternalListeners: function ContentDialog_addExternalListeners() {
_ElementUtilities._inputPaneListener.addEventListener(this._dom.root, "showing", this._onInputPaneShownBound);
_ElementUtilities._inputPaneListener.addEventListener(this._dom.root, "hiding", this._onInputPaneShownBound);
_ElementUtilities._inputPaneListener.addEventListener(this._dom.root, "hiding", this._onInputPaneHiddenBound);
},

_removeExternalListeners: function ContentDialog_removeExternalListeners() {
_ElementUtilities._inputPaneListener.removeEventListener(this._dom.root, "showing", this._onInputPaneShownBound);
_ElementUtilities._inputPaneListener.removeEventListener(this._dom.root, "hiding", this._onInputPaneShownBound);
_ElementUtilities._inputPaneListener.removeEventListener(this._dom.root, "hiding", this._onInputPaneHiddenBound);
},

_renderForInputPane: function ContentDialog_renderForInputPane(inputPaneHeight) {
this._clearInputPaneRendering();

var dialog = this._dom.dialog;
var style = dialog.style;
var left = dialog.offsetLeft;
var top = dialog.offsetTop;
var height = dialog.offsetHeight;
var bottom = top + height;
var visibleBottom = this._dom.root.offsetHeight - inputPaneHeight;
var titleHeight = _ElementUtilities.getTotalHeight(this._dom.title);
var commandsHeight = _ElementUtilities.getTotalHeight(this._dom.commandContainer);

if (bottom > visibleBottom) {
var newHeight = height - (bottom - visibleBottom);
if (newHeight - titleHeight - commandsHeight < minContentHeightWithInputPane) {

_shouldResizeForInputPane: function ContentDialog_shouldResizeForInputPane() {
if (this._rendered.resizedForInputPane) {
// Dialog has already resized for the input pane so it should continue adjusting
// its size as the input pane occluded rect changes.
return true;
} else {
// Dialog is at its default size. It should maintain its size as long as the
// input pane doesn't occlude it. This makes it so the dialog visual doesn't
// jump unnecessarily when the input pane shows if it won't occlude the dialog.
var dialogRect = this._dom.dialog.getBoundingClientRect();
var willInputPaneOccludeDialog =
_KeyboardInfo._KeyboardInfo._visibleDocTop > dialogRect.top ||
_KeyboardInfo._KeyboardInfo._visibleDocBottom < dialogRect.bottom;

return willInputPaneOccludeDialog;
}
},

// This function assumes it's in an environment that supports -ms-device-fixed.
_renderForInputPane: function ContentDialog_renderForInputPane() {
var rendered = this._rendered;

if (!rendered.registeredForResize) {
_ElementUtilities._resizeNotifier.subscribe(this._dom.root, this._onUpdateInputPaneRenderingBound);
rendered.registeredForResize = true;
}

if (this._shouldResizeForInputPane()) {
// Lay the dialog out using its normal rules but restrict it to the *visible document*
// rather than the *visual viewport*.
// Essentially, the dialog should be in the center of the part of the app that the user
// can see regardless of the state of the input pane.

var top = _KeyboardInfo._KeyboardInfo._visibleDocTop + "px";
var bottom = _KeyboardInfo._KeyboardInfo._visibleDocBottomOffset + "px";

if (rendered.top !== top) {
this._dom.root.style.top = top;
rendered.top = top;
}

if (rendered.bottom !== bottom) {
this._dom.root.style.bottom = bottom;
rendered.bottom = bottom;
}

if (!rendered.resizedForInputPane) {
// Put title into scroller so there's more screen real estate for the content
this._dom.scroller.insertBefore(this._dom.title, this._dom.content);
this._dom.root.style.height = "auto"; // Height will be determined by setting top & bottom
// Ensure activeElement is scrolled into view
var activeElement = _Global.document.activeElement;
if (activeElement && this._dom.scroller.contains(activeElement)) {
activeElement.scrollIntoView();
}
rendered.resizedForInputPane = true;
}

this._dom.root.style.display = "block";
style.height = newHeight + "px";
style.position = "absolute";
style.left = left + "px";
style.top = top + "px";
style.minHeight = 0;

this._resizedForInputPane = true;
_Global.document.activeElement.focus(); // Ensure activeElement is scrolled into view
}
},

_clearInputPaneRendering: function ContentDialog_clearInputPaneRendering() {
if (this._resizedForInputPane) {
if (this._dom.title.parentNode !== this._dom.dialog) {
// Make sure the title isn't in the scroller
this._dom.dialog.insertBefore(this._dom.title, this._dom.scroller);
}

var style = this._dom.dialog.style;
this._dom.root.style.display = "";
style.height = "";
style.position = "";
style.left = "";
style.top = "";
style.minHeight = "";
this._resizedForInputPane = false;
var rendered = this._rendered;

if (rendered.registeredForResize) {
_ElementUtilities._resizeNotifier.unsubscribe(this._dom.root, this._onUpdateInputPaneRenderingBound);
rendered.registeredForResize = false;
}

if (rendered.top !== "") {
this._dom.root.style.top = "";
rendered.top = "";
}

if (rendered.bottom !== "") {
this._dom.root.style.bottom = "";
rendered.bottom = "";
}

if (rendered.resizedForInputPane) {
// Make sure the title isn't in the scroller
this._dom.dialog.insertBefore(this._dom.title, this._dom.scroller);
this._dom.root.style.height = "";
rendered.resizedForInputPane = false;
}
}
}, {
Expand Down
12 changes: 11 additions & 1 deletion src/js/WinJS/Utilities/_KeyboardInfo.ts
View file
Expand Up @@ -12,6 +12,13 @@ var _Constants = {
scrollTimeout: 150,
}

// Definiton of *Visible Document*:
// Some portions of this file refer to the *visible document* or *visibleDoc*. Generally speaking,
// this is the portion of the app that is visible to the user (factoring in optical zoom and input pane occlusion).
// Technically speaking, in most cases, this is equivalent to the *visual viewport*. The exception is
// when the input pane has shown without resizing the *visual viewport*. In this case, the *visible document*
// is the *visual viewport* height minus the input pane occlusion.

// This private module provides accurate metrics for the Visual Viewport and WWA's IHM offsets in Win10 WWA
// where "-ms-device-fixed" CSS positioning is supported. WinJS controls will also use this module for
// positoning themselves relative to the viewport in a web browser outside of WWA. Their preference is still
Expand Down Expand Up @@ -154,7 +161,10 @@ _KeyboardInfo = {
return _Constants.scrollTimeout;
},

// _layoutViewportCoords is used with elements that use position:fixed instead of position:-ms-device-fixed
// _layoutViewportCoords gives the top and bottom offset of the visible document for elements using
// position:fixed. Comparison with position:-ms-device-fixed helper:
// - Like -ms-device-fixed helper, takes into account input pane occlusion.
// - Unlike -ms-device-fixed helper, doesn't account for optical zoom.
get _layoutViewportCoords(): { visibleDocTop: number; visibleDocBottom: number } {
var topOffset = _Global.window.pageYOffset - _Global.document.documentElement.scrollTop;
var bottomOffset = _Global.document.documentElement.clientHeight - (topOffset + this._visibleDocHeight);
Expand Down
25 changes: 17 additions & 8 deletions src/less/styles-contentdialog.less
View file
Expand Up @@ -27,13 +27,23 @@
@dialogVerticallyCenteredThreshold: 640px;

&.win-contentdialog-verticalalignment {
z-index: 1005; /* Above AppBar and SettingsFlyout. Below Flyout. */
position: fixed;
top: 0;
left: 0;
width: 100vw;
right: 0;
height: 100vh;
overflow: hidden;

&.win-contentdialog-devicefixedsupported {
// We have to use different techniques for sizing the height due to browser bugs:
// - In Edge, height:100vh doesn't work properly on elements with
// position:-ms-device-fixed so we use top:0 and bottom:0.
// - In Chrome, using top:0 and bottom:0 breaks percentage heights within the
// flexbox so we use height:100vh.
position: -ms-device-fixed;
height: auto;
bottom: 0;
}

display: none;
&.win-contentdialog-visible {
Expand All @@ -48,8 +58,8 @@
position: absolute;
top: 0;
left: 0;
width: 100vw;
height: 100vh;
width: 100%;
height: 100%;
}

.win-contentdialog-dialog {
Expand All @@ -63,8 +73,8 @@
outline-width: 1px;
box-sizing: border-box;
padding: 18px 24px 24px 24px;

width: 100vw;
width: 100%;
min-width: @minWidth;
max-width: @maxWidth;

Expand All @@ -87,8 +97,7 @@
This element works by moving between flexbox columns as the window's height changes.
*/
.win-contentdialog-column0or1 {
height: 50vh;
#flex > .flex(@grow: 10000; @shrink: 0);
#flex > .flex(@grow: 10000; @shrink: 0; @basis: 50%);
width: 0;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ContentDialog/ContentDialogFocusTests.ts
View file
Expand Up @@ -54,9 +54,9 @@ module ContentDialogTests {

Helper.waitForFocusWithin(dialog.element, function () { dialog.show(); }).then(function () {
LiveUnit.Assert.areEqual(
dialog.element.querySelector(".customButton1"),
dialog.element.querySelector("." + ContentDialog._ClassNames.dialog),
document.activeElement,
"The first focusable element in the dialog's content should have received focus"
"The dialog itself should receive focus"
);
dialog.hide();
Helper.validateUnhandledErrors();
Expand Down

0 comments on commit f4a93a5

Please sign in to comment.