Unverified Commit 88769409 authored by Kyle Keating's avatar Kyle Keating Committed by GitHub

Add visibility hidden to iframe when closed (#2095)

Add visibility hidden to the sidebar iframe when its closed (and after it finishes animation). This prevents all interaction from the components of the sidebar.

Add an optional coerce method to the hostPageSetting so that any incoming values can be coerced if needed. This may be required for values from from via that may only be a string type.

openSidebar config value gets coerced to a boolean in the annotator settings (just like it does in the host-config).
parent ae349f3e
import settingsFrom from './settings'; import settingsFrom from './settings';
import { toBoolean } from '../../shared/type-coercions';
/** /**
* Reads the Hypothesis configuration from the environment. * Reads the Hypothesis configuration from the environment.
...@@ -28,6 +29,8 @@ export default function configFrom(window_) { ...@@ -28,6 +29,8 @@ export default function configFrom(window_) {
onLayoutChange: settings.hostPageSetting('onLayoutChange'), onLayoutChange: settings.hostPageSetting('onLayoutChange'),
openSidebar: settings.hostPageSetting('openSidebar', { openSidebar: settings.hostPageSetting('openSidebar', {
allowInBrowserExt: true, allowInBrowserExt: true,
// Coerce value to a boolean because it may come from via as a string
coerce: toBoolean,
}), }),
query: settings.query, query: settings.query,
requestConfigFromFrame: settings.hostPageSetting('requestConfigFromFrame'), requestConfigFromFrame: settings.hostPageSetting('requestConfigFromFrame'),
......
...@@ -173,17 +173,20 @@ export default function settingsFrom(window_) { ...@@ -173,17 +173,20 @@ export default function settingsFrom(window_) {
function hostPageSetting(name, options = {}) { function hostPageSetting(name, options = {}) {
const allowInBrowserExt = options.allowInBrowserExt || false; const allowInBrowserExt = options.allowInBrowserExt || false;
const hasDefaultValue = typeof options.defaultValue !== 'undefined'; const hasDefaultValue = typeof options.defaultValue !== 'undefined';
// Optional coerce method, or a no-op.
const coerceValue =
typeof options.coerce === 'function' ? options.coerce : name => name;
if (!allowInBrowserExt && isBrowserExtension(sidebarAppUrl())) { if (!allowInBrowserExt && isBrowserExtension(sidebarAppUrl())) {
return hasDefaultValue ? options.defaultValue : null; return hasDefaultValue ? options.defaultValue : null;
} }
if (configFuncSettings.hasOwnProperty(name)) { if (configFuncSettings.hasOwnProperty(name)) {
return configFuncSettings[name]; return coerceValue(configFuncSettings[name]);
} }
if (jsonConfigs.hasOwnProperty(name)) { if (jsonConfigs.hasOwnProperty(name)) {
return jsonConfigs[name]; return coerceValue(jsonConfigs[name]);
} }
if (hasDefaultValue) { if (hasDefaultValue) {
......
...@@ -51,16 +51,13 @@ describe('annotator.config.index', function () { ...@@ -51,16 +51,13 @@ describe('annotator.config.index', function () {
}); });
}); });
['assetRoot', 'subFrameIdentifier', 'openSidebar'].forEach(function ( ['assetRoot', 'subFrameIdentifier'].forEach(function (settingName) {
settingName
) {
it( it(
'reads ' + 'reads ' +
settingName + settingName +
' from the host page, even when in a browser extension', ' from the host page, even when in a browser extension',
function () { function () {
configFrom('WINDOW'); configFrom('WINDOW');
assert.calledWithExactly( assert.calledWithExactly(
fakeSettingsFrom().hostPageSetting, fakeSettingsFrom().hostPageSetting,
settingName, settingName,
...@@ -70,6 +67,18 @@ describe('annotator.config.index', function () { ...@@ -70,6 +67,18 @@ describe('annotator.config.index', function () {
); );
}); });
it('reads openSidebar from the host page, even when in a browser extension', function () {
configFrom('WINDOW');
sinon.assert.calledWith(
fakeSettingsFrom().hostPageSetting,
'openSidebar',
sinon.match({
allowInBrowserExt: true,
coerce: sinon.match.func,
})
);
});
['branding', 'services'].forEach(function (settingName) { ['branding', 'services'].forEach(function (settingName) {
it( it(
'reads ' + 'reads ' +
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
@use './highlights'; @use './highlights';
var.$base-font-size: 14px; var.$base-font-size: 14px;
$sidebar-collapse-transition-time: 150ms;
// Sidebar // Sidebar
.annotator-frame { .annotator-frame {
...@@ -38,6 +39,14 @@ var.$base-font-size: 14px; ...@@ -38,6 +39,14 @@ var.$base-font-size: 14px;
&.annotator-collapsed { &.annotator-collapsed {
margin-left: 0; margin-left: 0;
.h-sidebar-iframe {
// Add a transition when collapsing only. This serves to delay
// the effect until the sidebar finishes closing. Visibility is
// a boolean value and can not actually animate.
transition: visibility $sidebar-collapse-transition-time;
visibility: hidden;
}
} }
* { * {
...@@ -189,7 +198,8 @@ var.$base-font-size: 14px; ...@@ -189,7 +198,8 @@ var.$base-font-size: 14px;
@media screen and (min-width: 37.5em) { @media screen and (min-width: 37.5em) {
.annotator-frame { .annotator-frame {
transition: margin-left 0.15s cubic-bezier(0.55, 0, 0.2, 0.8); transition: margin-left $sidebar-collapse-transition-time
cubic-bezier(0.55, 0, 0.2, 0.8);
width: 428px; width: 428px;
margin-left: -428px; margin-left: -428px;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment