Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle keyboard-dependent focus outlines in Vuex #4847

Merged
merged 4 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions kolibri/core/assets/src/core-app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require('kolibri.urls').default.setUp();
require('purecss/build/base-min.css');
require('../styles/main.scss');
require('../styles/globalDynamicStyles');
require('./vuexModality');

// Required to setup Keen UI, should be imported only once in your project
require('keen-ui/src/bootstrap');
Expand Down
112 changes: 112 additions & 0 deletions kolibri/core/assets/src/core-app/vuexModality.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/* global kolibriGlobal */
/**
* Adapted from https://github.com/alice/modality
* Version: 1.0.2
*
* For Kolibri, this mirrors in Vuex the DOM updates done by modality.js in Keen-UI,
* which gives the code in theme.js easier access to the current modality.
*/

document.addEventListener('DOMContentLoaded', () => {
let hadKeyboardEvent = false;
const keyboardModalityWhitelist = [
'input:not([type])',
'input[type=text]',
'input[type=number]',
'input[type=date]',
'input[type=time]',
'input[type=datetime]',
'textarea',
'[role=textbox]',
'[supports-modality=keyboard]',
].join(',');

let isHandlingKeyboardThrottle;

const matcher = (() => {
const el = document.body;

if (el.matchesSelector) {
return el.matchesSelector;
}

if (el.webkitMatchesSelector) {
return el.webkitMatchesSelector;
}

if (el.mozMatchesSelector) {
return el.mozMatchesSelector;
}

if (el.msMatchesSelector) {
return el.msMatchesSelector;
}

/* eslint-disable-next-line */
console.error("Couldn't find any matchesSelector method on document.body.");
})();

const disableFocusRingByDefault = function() {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this function here? Presumably the Keen-UI helper is still doing this part?

const css = 'body:not([modality=keyboard]) :focus { outline: none; }';
const head = document.head || document.getElementsByTagName('head')[0];
const style = document.createElement('style');

style.type = 'text/css';
style.id = 'disable-focus-ring';

if (style.styleSheet) {
style.styleSheet.cssText = css;
} else {
style.appendChild(document.createTextNode(css));
}

head.insertBefore(style, head.firstChild);
};

const focusTriggersKeyboardModality = function(el) {
let triggers = false;

if (matcher) {
triggers =
matcher.call(el, keyboardModalityWhitelist) && matcher.call(el, ':not([readonly])');
}

return triggers;
};

disableFocusRingByDefault();

document.body.addEventListener(
'keydown',
() => {
hadKeyboardEvent = true;

if (isHandlingKeyboardThrottle) {
clearTimeout(isHandlingKeyboardThrottle);
}

isHandlingKeyboardThrottle = setTimeout(() => {
hadKeyboardEvent = false;
}, 100);
},
true
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this was probably just copied over during the port so fine to leave as-is. However heads-up that we have two throttling libraries already included:


document.body.addEventListener(
'focus',
e => {
if (hadKeyboardEvent || focusTriggersKeyboardModality(e.target)) {
kolibriGlobal.coreVue.vuex.store.commit('SET_MODALITY', 'keyboard');
Copy link
Member

Choose a reason for hiding this comment

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

Any reason we can't just import the store here, rather than referencing the global?
Should be able to do: import store from 'kolibri.coreVue.vuex.store'; and then just reference that, no?

}
},
true
);

document.body.addEventListener(
'blur',
() => {
kolibriGlobal.coreVue.vuex.store.commit('SET_MODALITY', null);
Copy link
Member

Choose a reason for hiding this comment

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

As above.

},
true
);
});
22 changes: 22 additions & 0 deletions kolibri/core/assets/src/state/modules/theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const initialState = {
'$core-grey': '#e0e0e0',

'$core-loading': '#03a9f4',
modality: null,
};

export default {
Expand Down Expand Up @@ -90,7 +91,25 @@ export default {
$coreLoading(state) {
return state['$core-loading'];
},
// Should only use these styles to outline stuff that will be focused
// on keyboard-tab-focus
$coreOutline(state) {
if (state.modality !== 'keyboard') {
return {
outline: 'none',
};
}

return {
outlineColor: darken(state['$core-action-light'], 0.1),
outlineStyle: 'solid',
outlineWidth: '3px',
outlineOffset: '4px',
};
},
// Should use this when the outline needs to be applied regardless
// of modality
$coreOutlineAnyModality(state) {
return {
outlineColor: darken(state['$core-action-light'], 0.1),
outlineStyle: 'solid',
Expand All @@ -106,5 +125,8 @@ export default {
RESET_THEME_VALUE(state, varName) {
state[varName] = initialState[varName];
},
SET_MODALITY(state, modality) {
state.modality = modality;
},
},
};
1 change: 0 additions & 1 deletion kolibri/core/assets/src/views/ImmersiveToolbar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@
...mapGetters([
'$coreGrey200',
'$coreGrey300',
'$coreOutline',
'$coreActionDark',
'$coreActionNormal',
'$coreTextDefault',
Expand Down
4 changes: 2 additions & 2 deletions kolibri/core/assets/src/views/KNavbar/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
};
},
computed: {
...mapGetters(['$coreActionNormal', '$coreOutline']),
...mapGetters(['$coreActionNormal', '$coreOutlineAnyModality']),
maxWidth() {
return this.enoughSpace ? this.elementWidth : this.elementWidth - 38 * 2;
},
Expand All @@ -76,7 +76,7 @@
},
scrollButton() {
return {
':hover': this.$coreOutline,
':hover': this.$coreOutlineAnyModality,
};
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`app bar component drop down user menu should show the language modal li
<div style="background-color: rgb(153, 97, 137);" navshown="true">
<div class="ui-toolbar app-bar ui-toolbar--type-clear ui-toolbar--text-color-white ui-toolbar--progress-position-bottom" style="height: 20px;">
<div class="ui-toolbar__left">
<div class="ui-toolbar__nav-icon"><button type="button" tabindex="0" class="keen-ui-icon-button keen-ui-icon-button--type-secondary keen-ui-icon-button--color-default keen-ui-icon-button--size-normal KeenUiIconButton-noKey-0_8x3dw5">
<div class="ui-toolbar__nav-icon"><button type="button" tabindex="0" class="keen-ui-icon-button keen-ui-icon-button--type-secondary keen-ui-icon-button--color-default keen-ui-icon-button--size-normal KeenUiIconButton-noKey-0_1e7m9ar">
<div class="keen-ui-icon-button-icon" style="color: rgb(153, 97, 137);">
<mat-svg name="menu" category="navigation" class="icon"></mat-svg>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`side nav component should be hidden if navShown is false 1`] = `
<div class="side-nav-wrapper" style="position: relative;">
<div class="side-nav" style="width: 100px; color: rgb(58, 58, 58); background-color: rgb(255, 255, 255); display: none;" name="side-nav">
<div class="side-nav-header" style="height: 20px; width: 100px; padding-top: 8px; background-color: rgb(58, 58, 58);"><button aria-label="Close navigation" type="button" tabindex="0" class="keen-ui-icon-button keen-ui-icon-button--type-secondary keen-ui-icon-button--color-white keen-ui-icon-button--size-large KeenUiIconButton-noKey-0_8x3dw5">
<div class="side-nav-header" style="height: 20px; width: 100px; padding-top: 8px; background-color: rgb(58, 58, 58);"><button aria-label="Close navigation" type="button" tabindex="0" class="keen-ui-icon-button keen-ui-icon-button--type-secondary keen-ui-icon-button--color-white keen-ui-icon-button--size-large KeenUiIconButton-noKey-0_1e7m9ar">
<div class="keen-ui-icon-button-icon" style="color: rgb(153, 97, 137);">
<mat-svg name="close" category="navigation" class="side-nav-header-close"></mat-svg>
</div>
Expand All @@ -28,7 +28,7 @@ exports[`side nav component should be hidden if navShown is false 1`] = `
<div class="side-nav-scrollable-area-footer-info">
<p>Kolibri testversion</p>
<p>© 2018 Learning Equality</p>
<p><a dir="auto" type="button" tabindex="0" class="privacy-link KButton-noKey-0_1pw22jz link">Usage and privacy
<p><a dir="auto" type="button" tabindex="0" class="privacy-link KButton-noKey-0_khtjim link">Usage and privacy
<!----></a></p>
</div>
</div>
Expand All @@ -50,7 +50,7 @@ exports[`side nav component should be hidden if navShown is false 1`] = `
exports[`side nav component should show nothing if no components are added and user is not logged in 1`] = `
<div class="side-nav-wrapper" style="position: relative;">
<div class="side-nav" style="width: 100px; color: rgb(58, 58, 58); background-color: rgb(255, 255, 255);" name="side-nav">
<div class="side-nav-header" style="height: 20px; width: 100px; padding-top: 8px; background-color: rgb(58, 58, 58);"><button aria-label="Close navigation" type="button" tabindex="0" class="keen-ui-icon-button keen-ui-icon-button--type-secondary keen-ui-icon-button--color-white keen-ui-icon-button--size-large KeenUiIconButton-noKey-0_8x3dw5">
<div class="side-nav-header" style="height: 20px; width: 100px; padding-top: 8px; background-color: rgb(58, 58, 58);"><button aria-label="Close navigation" type="button" tabindex="0" class="keen-ui-icon-button keen-ui-icon-button--type-secondary keen-ui-icon-button--color-white keen-ui-icon-button--size-large KeenUiIconButton-noKey-0_1e7m9ar">
<div class="keen-ui-icon-button-icon" style="color: rgb(153, 97, 137);">
<mat-svg name="close" category="navigation" class="side-nav-header-close"></mat-svg>
</div>
Expand All @@ -75,7 +75,7 @@ exports[`side nav component should show nothing if no components are added and u
<div class="side-nav-scrollable-area-footer-info">
<p>Kolibri testversion</p>
<p>© 2018 Learning Equality</p>
<p><a dir="auto" type="button" tabindex="0" class="privacy-link KButton-noKey-0_1pw22jz link">Usage and privacy
<p><a dir="auto" type="button" tabindex="0" class="privacy-link KButton-noKey-0_khtjim link">Usage and privacy
<!----></a></p>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<thead slot="thead">
<tr>
<th v-if="selectable" class="core-table-icon-col">
<th v-if="selectable" class="core-table-checkbox-col">
<KCheckbox
:label="selectAllLabel"
:showLabel="false"
Expand Down Expand Up @@ -34,7 +34,7 @@
v-for="user in users"
:key="user.id"
>
<td v-if="selectable" class="core-table-icon-col">
<td v-if="selectable" class="core-table-checkbox-col">
<KCheckbox
:label="userCheckboxLabel"
:showLabel="false"
Expand Down