mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(tasks): add sub-task submit button and commit-on-blur on touch (#8860)
Android/mobile soft keyboards hide the Enter key or deliver it as a composing keydown that onKeydown filters out (to protect CJK candidate confirmation), so the inline sub-task draft could not be committed at all: neither Enter, tapping away, nor any on-screen control saved the text. - Add an always-visible submit button beside the input as an accessible, discoverable commit target (mouse-clickable, screen-reader labelled). Its mousedown is preventDefaulted so a desktop click keeps input focus, commits, and keeps the field open for rapid entry — mirroring the main add-task bar. On touch the tap commits via the blur path below. - Commit the draft on blur, but only on touch (isTouchActive): the natural mobile "done" gesture is tapping away, and the soft-keyboard Enter is unreliable there (#8791). Desktop keeps click-away-to-cancel — Enter and the button are the reliable commit paths — so blur no longer silently creates a task, and a user can move to the button without the draft being committed out from under them. Escape still discards on all devices. Both commit paths read the live input value, so IME/predictive-text buffering no longer strands the text. Closes #8856. Supersedes the standalone #8791 commit-on-blur branch. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
420292e37c
commit
6236dc7458
5 changed files with 226 additions and 43 deletions
|
|
@ -107,7 +107,10 @@ test.describe('Subtask inline input', () => {
|
|||
await expect(subTask).toBeFocused();
|
||||
});
|
||||
|
||||
test('does not create a subtask when a typed draft loses focus', async ({
|
||||
// On desktop (this suite runs mouse-primary Chrome), click-away cancels the
|
||||
// draft — commit-on-blur is scoped to touch (#8791/#8856), where the
|
||||
// soft-keyboard Enter is unreliable. Desktop commits via Enter or the button.
|
||||
test('discards a typed draft when it loses focus on desktop', async ({
|
||||
page,
|
||||
workViewPage,
|
||||
taskPage,
|
||||
|
|
|
|||
|
|
@ -1,12 +1,33 @@
|
|||
<input
|
||||
#inputEl
|
||||
class="add-subtask-input e2e-add-subtask-input"
|
||||
[ngModel]="titleDraft()"
|
||||
(ngModelChange)="titleDraft.set($event)"
|
||||
[placeholder]="T.F.TASK.CMP.ADD_SUB_TASK_PLACEHOLDER | translate"
|
||||
[attr.aria-label]="T.F.TASK.CMP.ADD_SUB_TASK_PLACEHOLDER | translate"
|
||||
(blur)="onBlur()"
|
||||
(keydown)="onKeydown($event)"
|
||||
matInput
|
||||
spellcheck="false"
|
||||
/>
|
||||
<div class="add-subtask-wrapper">
|
||||
<input
|
||||
#inputEl
|
||||
class="add-subtask-input e2e-add-subtask-input"
|
||||
[ngModel]="titleDraft()"
|
||||
(ngModelChange)="titleDraft.set($event)"
|
||||
[placeholder]="T.F.TASK.CMP.ADD_SUB_TASK_PLACEHOLDER | translate"
|
||||
[attr.aria-label]="T.F.TASK.CMP.ADD_SUB_TASK_PLACEHOLDER | translate"
|
||||
(blur)="onBlur()"
|
||||
(keydown)="onKeydown($event)"
|
||||
matInput
|
||||
spellcheck="false"
|
||||
/>
|
||||
|
||||
<!-- Visible commit target (accessibility + discoverability + touch tap target).
|
||||
mousedown is preventDefaulted so a desktop click does not blur the input
|
||||
away before onSubmitClick commits — on touch the tap commits via onBlur
|
||||
instead (see onSubmitClick). No pointerdown/focus trickery beyond that. -->
|
||||
<!-- tabindex=-1: keyboard users commit with Enter in the field — tabbing to
|
||||
the button would blur the input and cancel the draft on desktop. The
|
||||
button stays in the a11y tree for pointer and screen-reader-by-touch use. -->
|
||||
<button
|
||||
type="button"
|
||||
mat-icon-button
|
||||
tabindex="-1"
|
||||
class="submit-btn e2e-add-subtask-submit"
|
||||
(click)="onSubmitClick()"
|
||||
(mousedown)="$event.preventDefault()"
|
||||
[attr.aria-label]="T.F.TASK.CMP.ADD_SUB_TASK | translate"
|
||||
>
|
||||
<mat-icon>add</mat-icon>
|
||||
</button>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -3,12 +3,18 @@
|
|||
margin-top: var(--s-quarter);
|
||||
}
|
||||
|
||||
.add-subtask-wrapper {
|
||||
position: relative;
|
||||
}
|
||||
|
||||
.add-subtask-input {
|
||||
display: block;
|
||||
width: 100%;
|
||||
min-height: 40px;
|
||||
box-sizing: border-box;
|
||||
padding: 0 var(--s2);
|
||||
// Right padding reserves room for the absolutely-positioned submit button
|
||||
// (40px icon-button at a 2px offset) so typed text never slides under it.
|
||||
padding: 0 44px 0 var(--s2);
|
||||
border: 1px solid var(--extra-border-color);
|
||||
border-radius: var(--task-border-radius);
|
||||
outline: 0;
|
||||
|
|
@ -22,3 +28,10 @@
|
|||
border-color: var(--palette-primary-500);
|
||||
}
|
||||
}
|
||||
|
||||
.submit-btn {
|
||||
position: absolute;
|
||||
top: 50%;
|
||||
right: var(--s-quarter);
|
||||
transform: translateY(-50%);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,9 @@ describe('AddSubtaskInputComponent', () => {
|
|||
const getInput = (): HTMLInputElement =>
|
||||
fixture.nativeElement.querySelector('input') as HTMLInputElement;
|
||||
|
||||
const getSubmitBtn = (): HTMLButtonElement =>
|
||||
fixture.nativeElement.querySelector('.e2e-add-subtask-submit') as HTMLButtonElement;
|
||||
|
||||
const setInputValue = (value: string): void => {
|
||||
const input = getInput();
|
||||
input.value = value;
|
||||
|
|
@ -38,6 +41,16 @@ describe('AddSubtaskInputComponent', () => {
|
|||
fixture.detectChanges();
|
||||
};
|
||||
|
||||
// The test env is mouse-primary (detect-it deviceType 'mouseOnly'), so
|
||||
// _shouldCommitOnBlur() is false by default — force it on to exercise the
|
||||
// touch commit-on-blur path (#8791).
|
||||
const forceTouch = (): void => {
|
||||
spyOn(
|
||||
component as unknown as { _shouldCommitOnBlur: () => boolean },
|
||||
'_shouldCommitOnBlur',
|
||||
).and.returnValue(true);
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
taskServiceSpy = jasmine.createSpyObj<TaskService>('TaskService', ['addSubTaskTo']);
|
||||
|
||||
|
|
@ -112,10 +125,12 @@ describe('AddSubtaskInputComponent', () => {
|
|||
expect(closeSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('closes without creating a subtask on blur with content', () => {
|
||||
it('discards a typed draft on blur on desktop (mouse-primary) and closes', () => {
|
||||
// Desktop keeps click-away-to-cancel: Enter and the submit button are the
|
||||
// reliable commit paths there, so blur must not silently create a task.
|
||||
const closeSpy = jasmine.createSpy('closed');
|
||||
component.closed.subscribe(closeSpy);
|
||||
setInputValue('Blurred subtask');
|
||||
setInputValue('Discarded on desktop');
|
||||
|
||||
getInput().dispatchEvent(new FocusEvent('blur'));
|
||||
fixture.detectChanges();
|
||||
|
|
@ -175,4 +190,92 @@ describe('AddSubtaskInputComponent', () => {
|
|||
|
||||
expect(taskServiceSpy.addSubTaskTo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('commit on blur — touch only (#8791)', () => {
|
||||
it('commits a typed draft on blur when touch is active', () => {
|
||||
forceTouch();
|
||||
const closeSpy = jasmine.createSpy('closed');
|
||||
component.closed.subscribe(closeSpy);
|
||||
setInputValue('Touch blur subtask');
|
||||
|
||||
getInput().dispatchEvent(new FocusEvent('blur'));
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(taskServiceSpy.addSubTaskTo).toHaveBeenCalledOnceWith('parent-1', {
|
||||
title: 'Touch blur subtask',
|
||||
});
|
||||
expect(closeSpy).toHaveBeenCalledOnceWith('blur');
|
||||
});
|
||||
|
||||
it('commits composition-buffered text on blur, not just Enter', () => {
|
||||
// The reporting device (GrapheneOS/Vanadium) never delivered a usable
|
||||
// Enter, and blur must read the live input value so IME-buffered text is
|
||||
// still saved.
|
||||
forceTouch();
|
||||
const input = getInput();
|
||||
input.dispatchEvent(new CompositionEvent('compositionstart'));
|
||||
input.value = 'Composed on blur';
|
||||
input.dispatchEvent(new InputEvent('input', { bubbles: true, isComposing: true }));
|
||||
fixture.detectChanges();
|
||||
expect(component.titleDraft()).toBe('');
|
||||
|
||||
input.dispatchEvent(new FocusEvent('blur'));
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(taskServiceSpy.addSubTaskTo).toHaveBeenCalledOnceWith('parent-1', {
|
||||
title: 'Composed on blur',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('submit button (#8856)', () => {
|
||||
it('always renders a submit button (desktop included, for accessibility)', () => {
|
||||
expect(getSubmitBtn()).toBeTruthy();
|
||||
});
|
||||
|
||||
it('keeps the submit button out of the tab order (Enter is the keyboard path)', () => {
|
||||
// Tabbing to the button would blur + cancel the draft on desktop; keyboard
|
||||
// users commit with Enter, so the button is pointer/screen-reader-only.
|
||||
expect(getSubmitBtn().getAttribute('tabindex')).toBe('-1');
|
||||
});
|
||||
|
||||
it('commits the draft and keeps the input open when the button is clicked', fakeAsync(() => {
|
||||
const closeSpy = jasmine.createSpy('closed');
|
||||
component.closed.subscribe(closeSpy);
|
||||
setInputValue(' Button subtask ');
|
||||
|
||||
getSubmitBtn().click();
|
||||
fixture.detectChanges();
|
||||
tick(100);
|
||||
|
||||
expect(taskServiceSpy.addSubTaskTo).toHaveBeenCalledOnceWith('parent-1', {
|
||||
title: 'Button subtask',
|
||||
});
|
||||
expect(getInput().value).toBe('');
|
||||
expect(document.activeElement).toBe(getInput());
|
||||
expect(closeSpy).not.toHaveBeenCalled();
|
||||
}));
|
||||
|
||||
it('preventDefaults the button mousedown so a desktop click keeps input focus', () => {
|
||||
const ev = new MouseEvent('mousedown', { bubbles: true, cancelable: true });
|
||||
getSubmitBtn().dispatchEvent(ev);
|
||||
|
||||
expect(ev.defaultPrevented).toBe(true);
|
||||
});
|
||||
|
||||
it('adds exactly one sub-task when a touch blur and the submit click both fire', () => {
|
||||
// Real touch tap ordering: the tap blurs the input (commit + close) before
|
||||
// the button click dispatches onSubmitClick. The synchronous value-clear
|
||||
// in _addSubtaskFromInput must keep this to a single add.
|
||||
forceTouch();
|
||||
setInputValue('Once only');
|
||||
|
||||
getInput().dispatchEvent(new FocusEvent('blur'));
|
||||
component.onSubmitClick();
|
||||
|
||||
expect(taskServiceSpy.addSubTaskTo).toHaveBeenCalledOnceWith('parent-1', {
|
||||
title: 'Once only',
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -11,15 +11,19 @@ import {
|
|||
viewChild,
|
||||
} from '@angular/core';
|
||||
import { FormsModule } from '@angular/forms';
|
||||
import { MatIconButton } from '@angular/material/button';
|
||||
import { MatIcon } from '@angular/material/icon';
|
||||
import { MatInput } from '@angular/material/input';
|
||||
import { TranslatePipe } from '@ngx-translate/core';
|
||||
import { T } from '../../../t.const';
|
||||
import { isTouchActive } from '../../../util/input-intent';
|
||||
import { TaskService } from '../task.service';
|
||||
|
||||
/**
|
||||
* Why the inline draft input closed. `escape` is a keyboard cancel, so the
|
||||
* host should return focus to the task row; `blur` means focus already moved
|
||||
* elsewhere and must not be stolen back.
|
||||
* Why the inline draft input closed. `escape` is a keyboard cancel (the draft
|
||||
* is discarded and focus returns to the task row); `blur` means focus already
|
||||
* moved elsewhere, so any pending draft is committed and focus is not stolen
|
||||
* back.
|
||||
*/
|
||||
export type AddSubtaskInputCloseReason = 'escape' | 'blur';
|
||||
|
||||
|
|
@ -28,7 +32,7 @@ export type AddSubtaskInputCloseReason = 'escape' | 'blur';
|
|||
templateUrl: './add-subtask-input.component.html',
|
||||
styleUrl: './add-subtask-input.component.scss',
|
||||
changeDetection: ChangeDetectionStrategy.OnPush,
|
||||
imports: [FormsModule, MatInput, TranslatePipe],
|
||||
imports: [FormsModule, MatInput, MatIconButton, MatIcon, TranslatePipe],
|
||||
})
|
||||
export class AddSubtaskInputComponent {
|
||||
private readonly _taskService = inject(TaskService);
|
||||
|
|
@ -83,33 +87,43 @@ export class AddSubtaskInputComponent {
|
|||
return;
|
||||
}
|
||||
|
||||
// On touch, the natural "done" gesture is tapping away, and the soft-keyboard
|
||||
// Enter is unreliable (several IME/WebView combos, e.g. GrapheneOS/Vanadium,
|
||||
// deliver it as a composing keydown that onKeydown ignores) with no
|
||||
// tab-to-button, so blur must save the draft (#8791/#8856). On desktop, Enter
|
||||
// and the submit button are reliable, so blur cancels — the long-standing
|
||||
// behaviour — rather than silently creating a task on click-away, and a user
|
||||
// can move to the button without the draft being committed out from under
|
||||
// them. Escape always discards: it sets _isClosedWithoutSubmit, so its
|
||||
// trailing blur is skipped by the guard above.
|
||||
if (this._shouldCommitOnBlur()) {
|
||||
this._addSubtaskFromInput();
|
||||
}
|
||||
this._close('blur');
|
||||
}
|
||||
|
||||
private _commit(): void {
|
||||
// Read the live DOM value rather than the titleDraft signal: Angular's
|
||||
// DefaultValueAccessor buffers ngModelChange during IME / predictive-text
|
||||
// composition, so the signal can still be empty when Enter is pressed
|
||||
// mid-composition (the composition only ends — and the signal only
|
||||
// updates — once a trailing space or punctuation is typed). The input
|
||||
// element itself always holds the current text. Enter that instead
|
||||
// *confirms* an IME candidate carries isComposing and is already filtered
|
||||
// out in onKeydown, so this only commits genuinely-entered text. The
|
||||
// signal is a defensive fallback for the impossible case of inputEl being
|
||||
// unresolved (_commit only runs from a keydown on the rendered input).
|
||||
const inputEl = this.inputEl()?.nativeElement;
|
||||
const title = (inputEl?.value ?? this.titleDraft()).trim();
|
||||
if (!title) {
|
||||
return;
|
||||
}
|
||||
/** Commit the draft on blur only on touch — see onBlur. Overridable in specs. */
|
||||
protected _shouldCommitOnBlur(): boolean {
|
||||
return isTouchActive();
|
||||
}
|
||||
|
||||
this._taskService.addSubTaskTo(this.parentId(), { title });
|
||||
this.titleDraft.set('');
|
||||
// Clear the element directly too: when composition buffering kept the
|
||||
// signal empty, it is already '' and re-setting it would not write the
|
||||
// cleared value back through the one-way [ngModel] binding.
|
||||
if (inputEl) {
|
||||
inputEl.value = '';
|
||||
/**
|
||||
* Submit button handler. The button's mousedown is preventDefaulted (template)
|
||||
* so on desktop the click does not blur the input away first; this commits and
|
||||
* keeps the field open for rapid entry, mirroring the main add-task bar. On
|
||||
* touch, tapping the button usually blurs the input and commits via onBlur, so
|
||||
* this then reads the already-cleared value and is a no-op. Reuses _commit,
|
||||
* which reads the live value, so it works mid-composition.
|
||||
*/
|
||||
onSubmitClick(): void {
|
||||
this._commit();
|
||||
}
|
||||
|
||||
private _commit(): void {
|
||||
// Enter that *confirms* an IME candidate carries isComposing and is already
|
||||
// filtered out in onKeydown, so this only runs for genuinely-entered text.
|
||||
if (!this._addSubtaskFromInput()) {
|
||||
return;
|
||||
}
|
||||
|
||||
this._isKeepingOpenAfterSubmit = true;
|
||||
|
|
@ -122,6 +136,35 @@ export class AddSubtaskInputComponent {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a sub-task from the current input text; returns whether one was added.
|
||||
*
|
||||
* Reads the live DOM value rather than the titleDraft signal: Angular's
|
||||
* DefaultValueAccessor buffers ngModelChange during IME / predictive-text
|
||||
* composition, so the signal can still be empty when the user submits
|
||||
* mid-composition (the composition only ends — and the signal only updates —
|
||||
* once a trailing space or punctuation is typed). The input element itself
|
||||
* always holds the current text. The signal is a defensive fallback for the
|
||||
* impossible case of inputEl being unresolved.
|
||||
*/
|
||||
private _addSubtaskFromInput(): boolean {
|
||||
const inputEl = this.inputEl()?.nativeElement;
|
||||
const title = (inputEl?.value ?? this.titleDraft()).trim();
|
||||
if (!title) {
|
||||
return false;
|
||||
}
|
||||
|
||||
this._taskService.addSubTaskTo(this.parentId(), { title });
|
||||
this.titleDraft.set('');
|
||||
// Clear the element directly too: when composition buffering kept the
|
||||
// signal empty, it is already '' and re-setting it would not write the
|
||||
// cleared value back through the one-way [ngModel] binding.
|
||||
if (inputEl) {
|
||||
inputEl.value = '';
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private _close(reason: AddSubtaskInputCloseReason): void {
|
||||
if (this._isClosedWithoutSubmit) {
|
||||
return;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue