-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Changes from 4 commits
d4da489
eb176de
f4adea3
fed34e7
8d354e1
41c3b29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably easier to do There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've used this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thats right but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahhh if that happens then yes I agree 😆 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
}, | ||
this._editorInputGroupElm | ||
); | ||
|
@@ -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(); | ||
|
@@ -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); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,20 @@ describe('Example 11 - Batch Editing', () => { | |
cy.get('h3').should('contain', 'Example 11 - Batch Editing'); | ||
}); | ||
|
||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be easier to simply use Ctrl+Backspace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. {ctrl}{backspace} didn't work for me. But I doubt there is any con with this approach. So if you're fine I'd just keep it that way There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok sure whatever works is fine, just thought the alternative was shorter but if it doesn't work then too bad There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the E2E test still fails though, should we use |
||
.type('1970-01-01') | ||
.type('{enter}'); | ||
|
||
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 0}px;"] > .slick-cell:nth(5)`) | ||
.should('contain', '1970-01-01'); | ||
}) | ||
|
||
describe('built-in operators', () => { | ||
it('should click on "Clear Local Storage" and expect to be back to original grid with all the columns', () => { | ||
cy.get('[data-test="clear-storage-btn"]') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a particular reason to add the
disabled: false
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just an oversite from a manual test. good catch