[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]

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;
 }

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) {
+  public handleUserKeydown(event:JQueryEventObject) {
-    if (this.inEditMode && event.which === keyCodes.ENTER) {
+    // 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) {

frontend/app/components/wp-edit-form/work-package-edit-form.ts

@@ -173,6 +173,7 @@ export class WorkPackageEditForm {
    */
   public submit():ng.IPromise<WorkPackageResourceInterface> {
     if (this.changeset.empty && !this.workPackage.isNew) {
+      this.closeEditFields();
       return this.$q.when(this.workPackage);
     }
 

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}}" />

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}}" />

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}}" />

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}}" />

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}}" />

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 = table.editing.startEditing(workPackage, classIdentifier);
-    const form = WorkPackageEditForm.createInContext(editContext, workPackage, false);
 
     // Get the position where the user clicked.
     const positionOffset = ClickPositionMapper.getPosition(evt);

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 = '';

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);
+  }
+}
+

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
 

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