diff options
| author | Julien Mougenot <jum@odoo.com> | 2020-06-19 07:06:32 +0000 |
|---|---|---|
| committer | Aaron Bohy <aab@odoo.com> | 2020-06-22 13:04:01 +0000 |
| commit | 6998452065044650665c5020e6b730aede690ec0 (patch) | |
| tree | ea5bd87ab198b1b9ca7278f8eb9567786b2c2186 /addons/web/static/src/js | |
| parent | c7964657ecbbad278cec73cf231d7eef55b44347 (diff) | |
[IMP] web: disable keyboard navigation with sample data
Previous commit introduces the sample data feature in list and
kanban views. When sample data is displayed, most parts of the
renderers can't be clicked on, to prevent the user from interacting
with sample records (e.g. to open a sample record in form view from
the list). This commit disables keyboard navigation as well, as
those sample records could still be accessed with the keyboard.
To do so, several refactorings have been done in views:
- _giveFocus function added on controllers (so that we can hook
on the moment where we want to focus the renderer, and prevent
it in sample mode), it kind of replaces giveFocus of ActionMixin
which wasn't use from the outside
- updateRendererState -> _updateRendererState as it is private
- _setState function added on controllers, to have a hook when we
set a new state (already existed, but only in kanban)
- _renderView function moved from Basic to AbstractRenderer, so
that we have a proper hook to deactivate clickable elements in
sample mode
Task 2232801
X-original-commit: 4482a03fe2f083f4f44fc6eb76d90ae22afbff91
Diffstat (limited to 'addons/web/static/src/js')
13 files changed, 173 insertions, 121 deletions
diff --git a/addons/web/static/src/js/chrome/action_mixin.js b/addons/web/static/src/js/chrome/action_mixin.js index 99f45405c72..82feba04102 100644 --- a/addons/web/static/src/js/chrome/action_mixin.js +++ b/addons/web/static/src/js/chrome/action_mixin.js @@ -155,11 +155,6 @@ odoo.define('web.ActionMixin', function (require) { }, /** - * Gives the focus to the action - */ - giveFocus: function () { }, - - /** * Renders the buttons to append, in most cases, to the control panel (in * the bottom left corner). When the action is rendered in a dialog, those * buttons might be moved to the dialog's footer. diff --git a/addons/web/static/src/js/views/abstract_controller.js b/addons/web/static/src/js/views/abstract_controller.js index 3df1d48410b..07c4138c54d 100644 --- a/addons/web/static/src/js/views/abstract_controller.js +++ b/addons/web/static/src/js/views/abstract_controller.js @@ -96,7 +96,7 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { if (this.withControlPanel) { this._updateControlPanelProps(this.initialState); this._controlPanelWrapper = new ComponentWrapper(this, ControlPanel, this.controlPanelProps); - this._controlPanelWrapper.env.bus.on('focus-view', this, () => this.renderer.giveFocus()); + this._controlPanelWrapper.env.bus.on('focus-view', this, () => this._giveFocus()); promises.push(this._controlPanelWrapper.mount(this.el, { position: 'first-child' })); } await Promise.all(promises); @@ -193,12 +193,6 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { return state; }, /** - * Gives the focus to the renderer - */ - giveFocus: function () { - this.renderer.giveFocus(); - }, - /** * The use of this method is discouraged. It is still snakecased, because * it currently is used in many templates, but we will move to a simpler * mechanism as soon as we can. @@ -246,7 +240,7 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { } await Promise.all([this.update(params, {}), searchPanelUpdateProm]); if (postponeRendering) { - return this.renderer._render(); + return this._updateRendererState(false); } }, /** @@ -261,7 +255,6 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { * @param {Object} params will be given to the model and to the renderer * @param {Object} [options={}] * @param {boolean} [options.reload=true] if true, the model will reload data - * * @returns {Promise} */ async update(params, options = {}) { @@ -272,7 +265,7 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { const localState = this.renderer.getLocalState(); const state = this.model.get(this.handle, { withSampleData: true }); const promises = [ - this.updateRendererState(state, params).then(() => { + this._updateRendererState(state, params).then(() => { this.renderer.setLocalState(localState); }), this._update(this.model.get(this.handle), params) @@ -280,19 +273,6 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { await this.dp.add(Promise.all(promises)); this.updateButtons(); }, - /** - * Update the state of the renderer (handle both Widget and Component - * renderers). - * - * @param {Object} state the model state - * @param {Object} params will be given to the model and to the renderer - */ - updateRendererState: function (state, params) { - if (this.renderer instanceof owl.Component) { - return this.renderer.update(state); - } - return this.renderer.updateState(state, params); - }, //-------------------------------------------------------------------------- // Private @@ -327,6 +307,16 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { return null; }, /** + * Gives the focus to the renderer if not in sample mode. + * + * @private + */ + _giveFocus() { + if (!this.model.isInSampleMode()) { + this.renderer.giveFocus(); + } + }, + /** * This method is the way a view can notifies the outside world that * something has changed. The main use for this is to update the url, for * example with a new id. @@ -472,6 +462,21 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { return this.updateControlPanel({ pager: pagingInfo }); } }, + /** + * Updates the state of the renderer (handle both Widget and Component + * renderers). + * + * @private + * @param {Object} state the model state + * @param {Object} [params={}] will be given to the model and to the renderer + * @return {Promise} + */ + _updateRendererState(state, params = {}) { + if (this.renderer instanceof owl.Component) { + return this.renderer.update(state); + } + return this.renderer.updateState(state, params); + }, //-------------------------------------------------------------------------- // Handlers @@ -551,7 +556,7 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { break; case 'down': ev.stopPropagation(); - this.giveFocus(); + this._giveFocus(); break; } }, diff --git a/addons/web/static/src/js/views/abstract_renderer.js b/addons/web/static/src/js/views/abstract_renderer.js index afaf275ab5b..e1ae3fe5370 100644 --- a/addons/web/static/src/js/views/abstract_renderer.js +++ b/addons/web/static/src/js/views/abstract_renderer.js @@ -9,10 +9,28 @@ odoo.define('web.AbstractRenderer', function (require) { var mvc = require('web.mvc'); +// Renderers may display sample data when there is no real data to display. In +// this case the data is displayed with opacity and can't be clicked. Moreover, +// we also want to prevent the user from accessing DOM elements with TAB +// navigation. This is the list of elements we won't allow to focus. +const FOCUSABLE_ELEMENTS = [ + // focusable by default + 'a', 'button', 'input', 'select', 'textarea', + // manually set + '[tabindex="0"]' +].map((sel) => `:scope ${sel}`).join(', '); + /** * @class AbstractRenderer */ return mvc.Renderer.extend({ + // Defines the elements suppressed when in demo data. This must be a list + // of DOM selectors matching view elements that will: + // 1. receive the 'o_sample_data_disabled' class (greyd out & no user events) + // 2. have themselves and any of their focusable children removed from the + // tab navigation + sampleDataTargets: [], + /** * @override * @param {string} [params.noContentHelp] @@ -29,12 +47,12 @@ return mvc.Renderer.extend({ * * @returns {Promise} */ - start: function () { + async start() { this.$el.addClass(this.arch.attrs.class); if (this.withSearchPanel) { this.$el.addClass('o_renderer_with_searchpanel'); } - return Promise.all([this._render(), this._super()]); + await Promise.all([this._render(), this._super()]); }, /** * Called each time the renderer is attached into the DOM. @@ -92,9 +110,11 @@ return mvc.Renderer.extend({ * if true, the method only updates the state without rerendering * @returns {Promise} */ - updateState: function (state, params) { - this.state = state; - return params.noRender ? Promise.resolve() : this._render(); + async updateState(state, params) { + this._setState(state); + if (!params.noRender) { + await this._render(); + } }, //-------------------------------------------------------------------------- @@ -102,14 +122,66 @@ return mvc.Renderer.extend({ //-------------------------------------------------------------------------- /** - * Render the view + * Renders the widget. This method can be overriden to perform actions + * before or after the view has been rendered. + * + * @private + * @returns {Promise} + */ + async _render() { + await this._renderView(); + this._suppressFocusableElements(); + }, + /** + * Actual rendering. This method is meant to be overridden by concrete + * renderers. * * @abstract * @private * @returns {Promise} */ - _render: function () { - return Promise.resolve(); + async _renderView() { }, + /** + * Assigns a new state to the renderer if not false. + * + * @private + * @param {any} [state=false] + */ + _setState(state = false) { + if (state !== false) { + this.state = state; + } + }, + /** + * Suppresses 'tabindex' property on any focusable element located inside + * root elements defined in the `this.sampleDataTargets` object and assigns + * the 'o_sample_data_disabled' class to these root elements. + * + * @private + * @see sampleDataTargets + */ + _suppressFocusableElements() { + if (!this.state.isSample) { + return; + } + const rootEls = []; + for (const selector of this.sampleDataTargets) { + rootEls.push(...this.el.querySelectorAll(`:scope ${selector}`)); + } + const focusableEls = new Set(rootEls); + for (const rootEl of rootEls) { + rootEl.classList.add('o_sample_data_disabled'); + for (const focusableEl of rootEl.querySelectorAll(FOCUSABLE_ELEMENTS)) { + focusableEls.add(focusableEl); + } + } + for (const focusableEl of focusableEls) { + focusableEl.setAttribute('tabindex', -1); + if (focusableEl.classList.contains('dropdown-item')) { + // Tells Bootstrap to ignore the dropdown item in keynav + focusableEl.classList.add('disabled'); + } + } }, }); diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js index 0849cf5508d..ec7255b9e60 100644 --- a/addons/web/static/src/js/views/basic/basic_controller.js +++ b/addons/web/static/src/js/views/basic/basic_controller.js @@ -134,12 +134,6 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { return []; }, /** - * Gives the focus to the renderer - */ - giveFocus:function() { - this.renderer.giveFocus(); - }, - /** * Returns true iff the given recordID (or the main recordID) is dirty. * * @param {string} [recordID] - default to main recordID @@ -761,7 +755,7 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { await this.model.resequence(this.modelName, resIDs, this.handle, options); this._updateControlPanel(); state = this.model.get(this.handle); - return this.renderer.updateState(state, { noRender: true }); + return this._updateRendererState(state, { noRender: true }); }, }); }, diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js index 8db6058f242..f90e27d23d8 100644 --- a/addons/web/static/src/js/views/basic/basic_renderer.js +++ b/addons/web/static/src/js/views/basic/basic_renderer.js @@ -142,7 +142,7 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { */ confirmChange: function (state, id, fields, ev) { var self = this; - this.state = state; + this._setState(state); var record = this._getRecord(id); if (!record) { return this._render().then(_.constant([])); @@ -646,25 +646,25 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { return modifiersData.evaluatedModifiers[record.id]; }, /** - * Render the view - * * @override - * @returns {Promise} */ - _render: function () { - var oldAllFieldWidgets = this.allFieldWidgets; + async _render() { + const oldAllFieldWidgets = this.allFieldWidgets; this.allFieldWidgets = {}; // TODO maybe merging allFieldWidgets and allModifiersData into "nodesData" in some way could be great this.allModifiersData = []; - var oldWidgets = this.widgets; + const oldWidgets = this.widgets; this.widgets = []; - return this._renderView().then(function () { - _.each(oldAllFieldWidgets, function (recordWidgets) { - _.each(recordWidgets, function (widget) { - widget.destroy(); - }); - }); - _.invoke(oldWidgets, 'destroy'); - }); + + await this._super(...arguments); + + for (const id in oldAllFieldWidgets) { + for (const widget of oldAllFieldWidgets[id]) { + widget.destroy(); + } + } + for (const widget of oldWidgets) { + widget.destroy(); + } }, /** * Instantiates the appropriate AbstractField specialization for the given @@ -776,22 +776,6 @@ var BasicRenderer = AbstractRenderer.extend(WidgetAdapterMixin, { .append($noContent); }, /** - * Actual rendering. Supposed to be overridden by concrete renderers. - * The basic responsabilities of _renderView are: - * - use the xml arch of the view to render a jQuery representation - * - instantiate a widget from the registry for each field in the arch - * - * Note that the 'state' field should contains all necessary information - * for the rendering. The field widgets should be as synchronous as - * possible. - * - * @abstract - * @returns {Promise} - */ - _renderView: function () { - return Promise.resolve(); - }, - /** * Instantiate custom widgets * * @private diff --git a/addons/web/static/src/js/views/calendar/calendar_renderer.js b/addons/web/static/src/js/views/calendar/calendar_renderer.js index be96cf25068..a76c7ce3825 100644 --- a/addons/web/static/src/js/views/calendar/calendar_renderer.js +++ b/addons/web/static/src/js/views/calendar/calendar_renderer.js @@ -567,11 +567,9 @@ return AbstractRenderer.extend({ /** * Render the calendar view, this is the main entry point. * - * @override method from AbstractRenderer - * @private - * @returns {Promise} + * @override */ - _render: function () { + async _renderView() { this.$('.o_calendar_view')[0].prepend(this.calendarElement); if (this._isInDOM) { this._renderCalendar(); @@ -590,9 +588,7 @@ return AbstractRenderer.extend({ $a.not('.ui-state-active').addClass('o_color'); }); - var filterProm = this._renderFilters(); - - return Promise.all([filterProm, this._super.apply(this, arguments)]); + await this._renderFilters(); }, /** * Render the specific code for the FullCalendar when it's in the DOM diff --git a/addons/web/static/src/js/views/form/form_renderer.js b/addons/web/static/src/js/views/form/form_renderer.js index 1927d688c2c..2bcb833c14e 100644 --- a/addons/web/static/src/js/views/form/form_renderer.js +++ b/addons/web/static/src/js/views/form/form_renderer.js @@ -286,6 +286,7 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, { * @returns {Promise} */ updateState: function (state, params) { + this._setState(state); this.mode = (params && 'mode' in params) ? params.mode : this.mode; // if fieldNames are given, we update the corresponding field widget. @@ -293,7 +294,7 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, { // confirmChange method if (params.fieldNames) { // only update the given fields - return this.confirmChange(state, state.id, params.fieldNames); + return this.confirmChange(this.state, this.state.id, params.fieldNames); } return this._super.apply(this, arguments); }, diff --git a/addons/web/static/src/js/views/graph/graph_renderer.js b/addons/web/static/src/js/views/graph/graph_renderer.js index 4def49a6d7e..52608e7b9cd 100644 --- a/addons/web/static/src/js/views/graph/graph_renderer.js +++ b/addons/web/static/src/js/views/graph/graph_renderer.js @@ -76,12 +76,6 @@ return AbstractRenderer.extend({ this.$tooltip = null; }, /** - * @override - */ - updateState: function () { - return this._super.apply(this, arguments); - }, - /** * Chart.js does not need the canvas to be in dom in order * to be able to work well. We could avoid the calls to on_attach_callback * and on_detach_callback. @@ -754,10 +748,8 @@ return AbstractRenderer.extend({ * immediately, then we render the chart when the widget is in the DOM. * * @override - * @private - * @returns {Promise} The _super promise is actually resolved immediately */ - _render: function () { + async _renderView() { if (this.chart) { this.chart.destroy(); } @@ -796,7 +788,6 @@ return AbstractRenderer.extend({ this._renderTitle(); } - return this._super.apply(this, arguments); }, /** * create bar chart. diff --git a/addons/web/static/src/js/views/kanban/kanban_controller.js b/addons/web/static/src/js/views/kanban/kanban_controller.js index ea45c58f253..f7028d93d08 100644 --- a/addons/web/static/src/js/views/kanban/kanban_controller.js +++ b/addons/web/static/src/js/views/kanban/kanban_controller.js @@ -364,7 +364,7 @@ var KanbanController = BasicController.extend({ _onButtonsKeyDown: function (ev) { switch(ev.keyCode) { case $.ui.keyCode.DOWN: - this.$('.o_kanban_record:first').focus(); + this._giveFocus(); } }, /** diff --git a/addons/web/static/src/js/views/kanban/kanban_renderer.js b/addons/web/static/src/js/views/kanban/kanban_renderer.js index 8152d899150..cfb511ef79b 100644 --- a/addons/web/static/src/js/views/kanban/kanban_renderer.js +++ b/addons/web/static/src/js/views/kanban/kanban_renderer.js @@ -98,6 +98,14 @@ var KanbanRenderer = BasicRenderer.extend({ events:_.extend({}, BasicRenderer.prototype.events || {}, { 'keydown .o_kanban_record' : '_onRecordKeyDown' }), + sampleDataTargets: [ + '.o_kanban_counter', + '.o_kanban_record', + '.o_kanban_toggle_fold', + '.o_column_folded', + '.o_column_archive_records', + '.o_column_unarchive_records', + ], /** * @override @@ -200,7 +208,7 @@ var KanbanRenderer = BasicRenderer.extend({ var column = this.widgets[index]; this.widgets[index] = newColumn; if (options && options.state) { - this.state = options.state; + this._setState(options.state); } return newColumn.appendTo(document.createDocumentFragment()).then(function () { var def; @@ -247,13 +255,6 @@ var KanbanRenderer = BasicRenderer.extend({ } return Promise.resolve(); }, - /** - * @override - */ - updateState: function (state) { - this._setState(state); - return this._super.apply(this, arguments); - }, //-------------------------------------------------------------------------- // Private @@ -271,7 +272,7 @@ var KanbanRenderer = BasicRenderer.extend({ }, /** * Tries to give focus to the previous card, and returns true if successful - * + * * @private * @param {DOMElement} currentColumn * @returns {boolean} @@ -498,16 +499,15 @@ var KanbanRenderer = BasicRenderer.extend({ /** * Sets the current state and updates some internal attributes accordingly. * - * @private - * @param {Object} state + * @override */ - _setState: function (state) { - this.state = state; + _setState: function () { + this._super(...arguments); - var groupByField = state.groupedBy[0]; + var groupByField = this.state.groupedBy[0]; var cleanGroupByField = this._cleanGroupByField(groupByField); - var groupByFieldAttrs = state.fields[cleanGroupByField]; - var groupByFieldInfo = state.fieldsInfo.kanban[cleanGroupByField]; + var groupByFieldAttrs = this.state.fields[cleanGroupByField]; + var groupByFieldInfo = this.state.fieldsInfo.kanban[cleanGroupByField]; // Deactivate the drag'n'drop if the groupedBy field: // - is a date or datetime since we group by month or // - is readonly (on the field attrs or in the view) @@ -536,7 +536,7 @@ var KanbanRenderer = BasicRenderer.extend({ grouped_by_m2o: this.groupedByM2O, grouped_by_date: grouped_by_date, relation: relation, - quick_create: this.quickCreateEnabled && viewUtils.isQuickCreateEnabled(state), + quick_create: this.quickCreateEnabled && viewUtils.isQuickCreateEnabled(this.state), }); this.createColumnEnabled = this.groupedByM2O && this.columnOptions.group_creatable; }, diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js index 51f95e5a9ff..b786673ec7c 100644 --- a/addons/web/static/src/js/views/list/list_controller.js +++ b/addons/web/static/src/js/views/list/list_controller.js @@ -246,10 +246,14 @@ var ListController = BasicController.extend({ break; case $.ui.keyCode.DOWN: e.preventDefault(); - self.renderer.giveFocus(); + self._giveFocus(); break; case $.ui.keyCode.TAB: - if (!e.shiftKey && e.target.classList.contains("btn-primary")) { + if ( + !e.shiftKey && + e.target.classList.contains("btn-primary") && + !self.model.isInSampleMode() + ) { e.preventDefault(); $createButton.tooltip('show'); } @@ -268,7 +272,7 @@ var ListController = BasicController.extend({ */ _confirmSave: function (id) { var state = this.model.get(this.handle); - return this.renderer.updateState(state, {noRender: true}) + return this._updateRendererState(state, { noRender: true }) .then(this._setMode.bind(this, 'readonly', id)); }, /** @@ -445,7 +449,7 @@ var ListController = BasicController.extend({ // and the list was multi-editable, we do not want to select // the next row. this.selectedRecords = []; - await this.renderer.updateState(state, { + await this._updateRendererState(state, { keepWidths: true, selectedRecords: [], }); diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js index 278a5f2e896..818be2f350a 100644 --- a/addons/web/static/src/js/views/list/list_editable_renderer.js +++ b/addons/web/static/src/js/views/list/list_editable_renderer.js @@ -223,7 +223,7 @@ ListRenderer.include({ var self = this; var oldData = this.state.data; - this.state = state; + this._setState(state); return this.confirmChange(state, id, fields, ev).then(function () { // If no record with 'id' can be found in the state, the // confirmChange method will have rerendered the whole view already, @@ -369,7 +369,7 @@ ListRenderer.include({ * @param {string} recordID */ removeLine: function (state, recordID) { - this.state = state; + this._setState(state); var $row = this._getRow(recordID); if ($row.length === 0) { return; diff --git a/addons/web/static/src/js/views/list/list_renderer.js b/addons/web/static/src/js/views/list/list_renderer.js index 428a781319f..b35c42810a8 100644 --- a/addons/web/static/src/js/views/list/list_renderer.js +++ b/addons/web/static/src/js/views/list/list_renderer.js @@ -50,6 +50,12 @@ var ListRenderer = BasicRenderer.extend({ 'keydown td': '_onKeyDown', 'keydown th': '_onKeyDown', }, + sampleDataTargets: [ + '.o_data_row', + '.o_group_header', + '.o_list_table > tfoot', + '.o_list_table > thead .o_list_record_selector', + ], /** * @constructor * @param {Widget} parent @@ -90,8 +96,8 @@ var ListRenderer = BasicRenderer.extend({ /** * Order to focus to be given to the content of the current view + * * @override - * @public */ giveFocus: function () { this.$('th:eq(0) input, th:eq(1)').first().focus(); @@ -100,7 +106,8 @@ var ListRenderer = BasicRenderer.extend({ * @override */ updateState: function (state, params) { - this.isGrouped = state.groupedBy.length > 0; + this._setState(state); + this.isGrouped = this.state.groupedBy.length > 0; this._processColumns(params.columnInvisibleFields || {}); if (params.selectedRecords) { this.selection = params.selectedRecords; @@ -1231,7 +1238,7 @@ var ListRenderer = BasicRenderer.extend({ optionalColumnsEnabled: this.optionalColumnsEnabled, }); this._processColumns(this.columnInvisibleFields || {}); - this._renderView().then(function () { + this._render().then(function () { self._onToggleOptionalColumnDropdown(ev); }); }, @@ -1263,6 +1270,9 @@ var ListRenderer = BasicRenderer.extend({ var $tr; var $futureCell; var colIndex; + if (this.state.isSample) { + return; // we disable keyboard navigation inside the table in "sample" mode + } switch (ev.keyCode) { case $.ui.keyCode.LEFT: ev.preventDefault(); @@ -1345,7 +1355,7 @@ var ListRenderer = BasicRenderer.extend({ offset: currentMinimum - 1, on_success: reloadedGroup => { Object.assign(group, reloadedGroup); - this._renderView(); + this._render(); }, }); }, |
