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

feat: allow overriding readOnly behavior of dateEditor, fixes #1684 #1685

Merged
merged 6 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ prepareGrid() {
}
```

On top of the default ones provided by Vanilla-Calendar there are also:

* hideClearButton (default: false): which toggles the visiblity of the clear button
* allowInput (default: false): which determines whether you can directly enter a date in the input element

#### Grid Option `defaultEditorOptions
You could also define certain options as a global level (for the entire grid or even all grids) by taking advantage of the `defaultEditorOptions` Grid Option. Note that they are set via the editor type as a key name (`autocompleter`, `date`, ...) and then the content is the same as `editorOptions` (also note that each key is already typed with the correct editor option interface), for example

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export default class Example11 {
type: FieldType.date, outputType: FieldType.dateIso,
filterable: true,
filter: { model: Filters.compoundDate },
editor: { model: Editors.date, massUpdate: true },
editor: { model: Editors.date, massUpdate: true, editorOptions: { allowInput: true } },
},
{
id: 'finish', name: 'Finish', field: 'finish', sortable: true, minWidth: 80,
Expand Down
45 changes: 45 additions & 0 deletions packages/common/src/editors/__tests__/dateEditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,34 @@ describe('DateEditor', () => {
expect(showSpy).toHaveBeenCalled();
});

it('should initialize the editor and add a keydown event listener that early exists by default', () => {
editor = new DateEditor(editorArguments);

const event = new KeyboardEvent('keydown', { key: 'Enter' });
editor.editorDomElement.dispatchEvent(event);

expect(editor.columnEditor.editorOptions?.allowEdit).toBeFalsy();
expect(editor.isValueTouched()).toBeFalsy();
});

it('should stop propagation on allowEdit when hitting left or right arrow keys', () => {
editor = new DateEditor({ ...editorArguments,
column: { ...editorArguments.column,
editor: { ...editorArguments.column.editor,
editorOptions: { ...editorArguments.column?.editor?.editorOptions, allowEdit: true }
}}});

let event = new KeyboardEvent('keydown', { key: 'ArrowLeft' });
let propagationSpy = vi.spyOn(event, 'stopImmediatePropagation');
editor.editorDomElement.dispatchEvent(event);
expect(propagationSpy).toHaveBeenCalled();

event = new KeyboardEvent('keydown', { key: 'ArrowRight' });
propagationSpy = vi.spyOn(event, 'stopImmediatePropagation');
editor.editorDomElement.dispatchEvent(event);
expect(propagationSpy).toHaveBeenCalled();
});

it('should have a placeholder when defined in its column definition', () => {
const testValue = 'test placeholder';
mockColumn.editor!.placeholder = testValue;
Expand Down Expand Up @@ -267,6 +295,23 @@ describe('DateEditor', () => {
expect(editor.isValueTouched()).toBe(true);
});

it('should return True when the last key was enter and alwaysSaveOnEnterKey is active', () => {
mockItemData = { id: 1, startDate: '2001-01-02T11:02:02.000Z', isActive: true };

editor = new DateEditor({...editorArguments,
column: { ...mockColumn, editor: { ...editorArguments.column.editor, alwaysSaveOnEnterKey: true,
editorOptions: { ...editorArguments.column.editor?.editorOptions, allowEdit: true}
} }
});
const event = new KeyboardEvent('keydown', { key: 'Enter' });
vi.runAllTimers();

editor.editorDomElement.dispatchEvent(event);

expect(editor.isValueChanged()).toBe(true);

});

it('should return True when date is reset by the clear date button', () => {
mockItemData = { id: 1, startDate: '2001-01-02T11:02:02.000Z', isActive: true };

Expand Down
21 changes: 20 additions & 1 deletion packages/common/src/editors/dateEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class DateEditor implements Editor {
protected _lastTriggeredByClearDate = false;
protected _originalDate?: string;
protected _pickerMergedOptions!: IOptions;
protected _lastInputKeyEvent?: KeyboardEvent;
calendarInstance?: VanillaCalendar;
defaultDate?: string;
hasTimePicker = false;
Expand Down Expand Up @@ -185,7 +186,7 @@ export class DateEditor implements Editor {
title: this.columnEditor && this.columnEditor.title || '',
className: inputCssClasses.replace(/\./g, ' '),
dataset: { input: '', defaultdate: this.defaultDate },
readOnly: true,
readOnly: this.columnEditor.editorOptions?.allowEdit === true ? true : false,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably easier to do !!(this.columnEditor.editorOptions?.allowEdit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing on options to other options I'm typically careful since true is not the same as truthy but you're probably right here

Copy link
Owner

@ghiscoding ghiscoding Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used this !!(var) for years, it's an easy shortcut to convert the condition to a boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats right but "false" == true and can happen relatively easy as consumer of the grid. chances are the user meant false just mistankely added quotes

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh if that happens then yes I agree 😆

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zewa666 ah this is probably why it fails, it looks like you forgot rename this readonly to allowInput

},
this._editorInputGroupElm
);
Expand All @@ -202,6 +203,18 @@ export class DateEditor implements Editor {
});
}

this._bindEventService.bind(this._inputElm, 'keydown', ((event: KeyboardEvent) => {
if (this.columnEditor.editorOptions?.allowEdit !== true) {
return;
}

this._isValueTouched = true;
this._lastInputKeyEvent = event;
if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') {
event.stopImmediatePropagation();
}
}) as EventListener);

queueMicrotask(() => {
this.calendarInstance = new VanillaCalendar(this._inputElm, this._pickerMergedOptions);
this.calendarInstance.init();
Expand Down Expand Up @@ -349,6 +362,12 @@ export class DateEditor implements Editor {
let isChanged = false;
const elmDateStr = this.getValue();

const lastEventKey = this._lastInputKeyEvent?.key;
if (this.columnEditor.editorOptions?.allowEdit === true &&
this.columnEditor?.alwaysSaveOnEnterKey && lastEventKey === 'Enter') {
return true;
}

if (this.columnDef) {
isChanged = this._lastTriggeredByClearDate || (!(elmDateStr === '' && this._originalDate === '')) && (elmDateStr !== this._originalDate);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ export interface VanillaCalendarOption extends IPartialSettings {

/** defaults to false, do we want to hide the clear date button? */
hideClearButton?: boolean;

/** defaults to false, should keyboard entries be allowed in input field? */
allowInput?: boolean;
}
16 changes: 16 additions & 0 deletions test/cypress/e2e/example11.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1087,4 +1087,20 @@ describe('Example 11 - Batch Editing', () => {
.click({ force: true });
});
});

describe('with Date Editor', () => {
it('should input values directly in input of datepicker', () => {
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 0}px;"] > .slick-cell:nth(5)`)
.click();

cy.get(`.input-group-editor`)
.focus()
.type('{backspace}'.repeat(10))
.type('1970-01-01')
.type('{enter}');

cy.get(`[style="top: ${GRID_ROW_HEIGHT * 0}px;"] > .slick-cell:nth(5)`)
.should('contain', '1970-01-01');
});
});
});
Loading