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 3 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 @@ -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