From 3a6f2e227832bb15885995e1aba39102c1e74f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 7 Aug 2017 09:13:30 +0200 Subject: [PATCH] [26044] Handle escape in field handler and reference form in table (#5828) * [26044] Handle escape in field handler and reference form in table Mousetrap was used to trigger escape on the field, but since we already have a keydown handler attached to it, we can re-use it for cancelling the field editing. While fixing this, I found out that the table edit context uses a new form for each field, which means the edit handler of the previous field is lost. A new reference context is held in the table to return existing forms, if any. In the single view, this is tied to the $scope of the edit field group, and thus ensure only one form exists. * Fix specs of long text fields now no longer escape-able. [ci skip] --- .../wp-edit-form/work-package-edit-context.ts | 5 --- .../work-package-edit-field-handler.ts | 32 ++++++++------ .../wp-edit-form/work-package-edit-form.ts | 1 + .../wp-edit-date-field.directive.html | 2 +- .../wp-edit-duration-field.directive.html | 2 +- .../wp-edit-float-field.directive.html | 2 +- .../wp-edit-integer-field.directive.html | 2 +- .../wp-edit-text-field.directive.html | 2 +- .../handlers/cell/edit-cell-handler.ts | 5 ++- .../components/wp-fast-table/wp-fast-table.ts | 6 +++ .../wp-fast-table/wp-table-editing.ts | 44 +++++++++++++++++++ .../inplace_editor/custom_field_spec.rb | 14 +++++- .../inplace_editor/description_editor_spec.rb | 12 ++++- 13 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 frontend/app/components/wp-fast-table/wp-table-editing.ts diff --git a/frontend/app/components/wp-edit-form/work-package-edit-context.ts b/frontend/app/components/wp-edit-form/work-package-edit-context.ts index 948902b7d9..0591cf25cd 100644 --- a/frontend/app/components/wp-edit-form/work-package-edit-context.ts +++ b/frontend/app/components/wp-edit-form/work-package-edit-context.ts @@ -58,9 +58,4 @@ export interface WorkPackageEditContext { * Return the first relevant field from the given list of attributes. */ firstField(names:string[]):string; - - /** - * ui-state to redirect to after successful saving - */ - successState:string; } diff --git a/frontend/app/components/wp-edit-form/work-package-edit-field-handler.ts b/frontend/app/components/wp-edit-form/work-package-edit-field-handler.ts index ed5493a0e3..d34c425b80 100644 --- a/frontend/app/components/wp-edit-form/work-package-edit-field-handler.ts +++ b/frontend/app/components/wp-edit-form/work-package-edit-field-handler.ts @@ -64,17 +64,6 @@ export class WorkPackageEditFieldHandler { if (withErrors !== undefined) { this.setErrors(withErrors); } - - - - Mousetrap(element[0]).bind('escape', () => { - if (!this.inEditMode) { - this.handleUserCancel(); - return false; - } - - return true; - }); } /** @@ -117,10 +106,25 @@ export class WorkPackageEditFieldHandler { * In an edit mode, we can't derive from a submit event wheteher the user pressed enter * (and on what field he did that). */ - public handleUserSubmitOnEnter(event:JQueryEventObject) { - if (this.inEditMode && event.which === keyCodes.ENTER) { - this.form.submit(); + public handleUserKeydown(event:JQueryEventObject) { + // Only handle submission in edit mode + if (this.inEditMode) { + if (event.which === keyCodes.ENTER) { + this.form.submit(); + return false; + } + return true; } + + // Escape editing when not in edit mode + if (event.which === keyCodes.ESCAPE) { + this.handleUserCancel(); + return false; + } + + // If enter is pressed here, it will continue to handleUserSubmit() + // due to the form submission event. + return true; } public onlyInAccessibilityMode(callback:Function) { diff --git a/frontend/app/components/wp-edit-form/work-package-edit-form.ts b/frontend/app/components/wp-edit-form/work-package-edit-form.ts index eec85a3f21..3b17612a8b 100644 --- a/frontend/app/components/wp-edit-form/work-package-edit-form.ts +++ b/frontend/app/components/wp-edit-form/work-package-edit-form.ts @@ -173,6 +173,7 @@ export class WorkPackageEditForm { */ public submit():ng.IPromise { if (this.changeset.empty && !this.workPackage.isNew) { + this.closeEditFields(); return this.$q.when(this.workPackage); } diff --git a/frontend/app/components/wp-edit/field-types/wp-edit-date-field.directive.html b/frontend/app/components/wp-edit/field-types/wp-edit-date-field.directive.html index 0e6a477891..1d419f4b30 100644 --- a/frontend/app/components/wp-edit/field-types/wp-edit-date-field.directive.html +++ b/frontend/app/components/wp-edit/field-types/wp-edit-date-field.directive.html @@ -8,7 +8,7 @@ class="wp-inline-edit--field" transform-date-value ng-blur="vm.onlyInAccessibilityMode(vm.handleUserBlur)" - ng-keydown="vm.handleUserSubmitOnEnter($event)" + ng-keydown="vm.handleUserKeydown($event)" ng-required="vm.field.required" ng-disabled="vm.field.inFlight" ng-attr-id="{{vm.htmlId}}" /> diff --git a/frontend/app/components/wp-edit/field-types/wp-edit-duration-field.directive.html b/frontend/app/components/wp-edit/field-types/wp-edit-duration-field.directive.html index 1c764c0435..14bc2c64dc 100644 --- a/frontend/app/components/wp-edit/field-types/wp-edit-duration-field.directive.html +++ b/frontend/app/components/wp-edit/field-types/wp-edit-duration-field.directive.html @@ -7,6 +7,6 @@ ng-required="vm.field.required" ng-focus="vm.handleUserFocus()" ng-blur="vm.handleUserBlur()" - ng-keydown="vm.handleUserSubmitOnEnter($event)" + ng-keydown="vm.handleUserKeydown($event)" ng-disabled="vm.field.inFlight" ng-attr-id="{{vm.htmlId}}" /> diff --git a/frontend/app/components/wp-edit/field-types/wp-edit-float-field.directive.html b/frontend/app/components/wp-edit/field-types/wp-edit-float-field.directive.html index 945aada9ff..2cfc4b6e80 100644 --- a/frontend/app/components/wp-edit/field-types/wp-edit-float-field.directive.html +++ b/frontend/app/components/wp-edit/field-types/wp-edit-float-field.directive.html @@ -5,7 +5,7 @@ ng-required="vm.field.required" ng-focus="vm.handleUserFocus()" ng-blur="vm.handleUserBlur()" - ng-keydown="vm.handleUserSubmitOnEnter($event)" + ng-keydown="vm.handleUserKeydown($event)" transform-float-value ng-disabled="vm.field.inFlight" ng-attr-id="{{vm.htmlId}}" /> diff --git a/frontend/app/components/wp-edit/field-types/wp-edit-integer-field.directive.html b/frontend/app/components/wp-edit/field-types/wp-edit-integer-field.directive.html index cdba1d3fef..cee6123c0d 100644 --- a/frontend/app/components/wp-edit/field-types/wp-edit-integer-field.directive.html +++ b/frontend/app/components/wp-edit/field-types/wp-edit-integer-field.directive.html @@ -5,6 +5,6 @@ ng-required="vm.field.required" ng-focus="vm.handleUserFocus()" ng-blur="vm.handleUserBlur()" - ng-keydown="vm.handleUserSubmitOnEnter($event)" + ng-keydown="vm.handleUserKeydown($event)" ng-disabled="vm.field.inFlight" ng-attr-id="{{vm.htmlId}}" /> diff --git a/frontend/app/components/wp-edit/field-types/wp-edit-text-field.directive.html b/frontend/app/components/wp-edit/field-types/wp-edit-text-field.directive.html index 48d8a28100..f811f01902 100644 --- a/frontend/app/components/wp-edit/field-types/wp-edit-text-field.directive.html +++ b/frontend/app/components/wp-edit/field-types/wp-edit-text-field.directive.html @@ -5,6 +5,6 @@ ng-required="vm.field.required" ng-focus="vm.handleUserFocus()" ng-disabled="vm.field.inFlight" - ng-keydown="vm.handleUserSubmitOnEnter($event)" + ng-keydown="vm.handleUserKeydown($event)" focus="vm.field.name === 'subject'" ng-attr-id="{{vm.htmlId}}" /> diff --git a/frontend/app/components/wp-fast-table/handlers/cell/edit-cell-handler.ts b/frontend/app/components/wp-fast-table/handlers/cell/edit-cell-handler.ts index 9a04060de6..5cc720ba5e 100644 --- a/frontend/app/components/wp-fast-table/handlers/cell/edit-cell-handler.ts +++ b/frontend/app/components/wp-fast-table/handlers/cell/edit-cell-handler.ts @@ -20,6 +20,8 @@ export class EditCellHandler extends ClickOrEnterHandler implements TableEventHa public states:States; public wpEditing:WorkPackageEditingService; + // Keep a reference to all + public get EVENT() { return 'click.table.cell, keydown.table.cell'; } @@ -60,8 +62,7 @@ export class EditCellHandler extends ClickOrEnterHandler implements TableEventHa const classIdentifier = rowElement.data('classIdentifier'); // Get any existing edit state for this work package - const editContext = new TableRowEditContext(workPackageId, classIdentifier); - const form = WorkPackageEditForm.createInContext(editContext, workPackage, false); + const form = table.editing.startEditing(workPackage, classIdentifier); // Get the position where the user clicked. const positionOffset = ClickPositionMapper.getPosition(evt); diff --git a/frontend/app/components/wp-fast-table/wp-fast-table.ts b/frontend/app/components/wp-fast-table/wp-fast-table.ts index ae30c438f3..841d204cd4 100644 --- a/frontend/app/components/wp-fast-table/wp-fast-table.ts +++ b/frontend/app/components/wp-fast-table/wp-fast-table.ts @@ -17,6 +17,7 @@ import {RowsBuilder} from './builders/modes/rows-builder'; import {WorkPackageTimelineTableController} from '../wp-table/timeline/container/wp-timeline-container.directive'; import {PrimaryRenderPass, RenderedRow} from './builders/primary-render-pass'; import {debugLog} from '../../helpers/debug_output'; +import {WorkPackageTableEditingContext} from "./wp-table-editing"; export class WorkPackageTable { public wpCacheService:WorkPackageCacheService; @@ -37,6 +38,10 @@ export class WorkPackageTable { // Last render pass used for refreshing single rows private lastRenderPass:PrimaryRenderPass|null = null; + // Work package editing context handler in the table, which handles open forms + // and their contexts + public editing:WorkPackageTableEditingContext = new WorkPackageTableEditingContext(); + constructor(public container:HTMLElement, public tbody:HTMLElement, public timelineBody:HTMLElement, @@ -105,6 +110,7 @@ export class WorkPackageTable { * Redraw all elements in the table section only */ public redrawTable() { + this.editing.reset(); const renderPass = this.lastRenderPass = this.rowBuilder.buildRows(); this.tbody.innerHTML = ''; diff --git a/frontend/app/components/wp-fast-table/wp-table-editing.ts b/frontend/app/components/wp-fast-table/wp-table-editing.ts new file mode 100644 index 0000000000..42e3da08fc --- /dev/null +++ b/frontend/app/components/wp-fast-table/wp-table-editing.ts @@ -0,0 +1,44 @@ +import {WorkPackageCacheService} from '../work-packages/work-package-cache.service'; +import { + WorkPackageResource, + WorkPackageResourceInterface +} from '../api/api-v3/hal-resources/work-package-resource.service'; + +import {States} from '../states.service'; +import {injectorBridge} from '../angular/angular-injector-bridge.functions'; + +import {WorkPackageTableRow} from './wp-table.interfaces'; +import {TableHandlerRegistry} from './handlers/table-handler-registry'; +import {locateRow} from './helpers/wp-table-row-helpers'; +import {PlainRowsBuilder} from './builders/modes/plain/plain-rows-builder'; +import {GroupedRowsBuilder} from './builders/modes/grouped/grouped-rows-builder'; +import {HierarchyRowsBuilder} from './builders/modes/hierarchy/hierarchy-rows-builder'; +import {RowsBuilder} from './builders/modes/rows-builder'; +import {WorkPackageTimelineTableController} from '../wp-table/timeline/container/wp-timeline-container.directive'; +import {PrimaryRenderPass, RenderedRow} from './builders/primary-render-pass'; +import {debugLog} from '../../helpers/debug_output'; +import {WorkPackageEditForm} from "../wp-edit-form/work-package-edit-form"; +import {TableRowEditContext} from "../wp-edit-form/table-row-edit-context"; + +export class WorkPackageTableEditingContext { + + public forms:{[wpId:string]:WorkPackageEditForm} = {}; + + public reset() { + _.each(this.forms, (form) => form.destroy()); + this.forms = {}; + } + + public startEditing(workPackage:WorkPackageResourceInterface, classIdentifier:string):WorkPackageEditForm { + const wpId = workPackage.id; + const existing = this.forms[wpId]; + if (existing) { + return existing; + } + + // Get any existing edit state for this work package + const editContext = new TableRowEditContext(wpId, classIdentifier); + return this.forms[wpId] = WorkPackageEditForm.createInContext(editContext, workPackage, false); + } +} + diff --git a/spec/features/work_packages/details/inplace_editor/custom_field_spec.rb b/spec/features/work_packages/details/inplace_editor/custom_field_spec.rb index 8ed5c65455..81ca690d95 100644 --- a/spec/features/work_packages/details/inplace_editor/custom_field_spec.rb +++ b/spec/features/work_packages/details/inplace_editor/custom_field_spec.rb @@ -45,8 +45,20 @@ describe 'custom field inplace editor', js: true do let(:initial_custom_values) { { custom_field.id => 'foo' } } let(:field) { WorkPackageTextAreaField.new wp_page, :customField1 } + it 'can cancel through the button only' do + # Activate the field + field.activate! + + # Pressing escape does nothing here + field.cancel_by_escape + field.expect_active! + + # Cancelling through the action panel + field.cancel_by_click + field.expect_inactive! + end + it_behaves_like 'a previewable field' - it_behaves_like 'a cancellable field' it_behaves_like 'an autocomplete field' end diff --git a/spec/features/work_packages/details/inplace_editor/description_editor_spec.rb b/spec/features/work_packages/details/inplace_editor/description_editor_spec.rb index e28087cb80..93bf5cfd25 100644 --- a/spec/features/work_packages/details/inplace_editor/description_editor_spec.rb +++ b/spec/features/work_packages/details/inplace_editor/description_editor_spec.rb @@ -45,6 +45,17 @@ describe 'description inplace editor', js: true, selenium: true do field.activate! field.expect_value description_text field.cancel_by_click + + # Activate the field + field.activate! + + # Pressing escape does nothing here + field.cancel_by_escape + field.expect_active! + + # Cancelling through the action panel + field.cancel_by_click + field.expect_inactive! end end @@ -86,6 +97,5 @@ describe 'description inplace editor', js: true, selenium: true do end end - it_behaves_like 'a cancellable field' it_behaves_like 'an autocomplete field' end