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

Fix date editor keyboard copy #416

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

pkalita-lbl
Copy link
Collaborator

I believe this is the necessary fix to get keyboard copy and paste working for date/datetime cells. In HT 8.0.0 they introduced a new mechanism to identify which elements are "owned" by HT. Previously, when we were using HT 7.4.x, adding the handsontableInput class was sufficient. Now they use an attribute called data-hot-input. See: handsontable/handsontable#6383. Without this identification the right document-level event listeners aren't attached when those cells are selected prior to copying.

Secondarily I updated the calls to a few HT helper methods to reflect the preferred ones in 13.1.x.

Lastly, unrelated to all of that I removed the erroneous package-lock.json file and updated yarn.lock to reflect the current dependencies. If we want to switch to using npm for this project instead of yarn I have no strong feelings, but we shouldn't mix the two.

@kennethbruskiewicz
Copy link
Collaborator

Taking a look.

@kennethbruskiewicz
Copy link
Collaborator

I could be wrong but there are no templates in DataHarmonizer right now that take "xsd:datetime" and "xsd:time" as ranges, only "date". That said the copy-paste function did work for me when testing ambr/AMBR's "sample received date" slot, after picking a date in the control.

The edits look good, will merge.

@pkalita-lbl is there anything about how the HOT editors currently work which should also apply to the other editors we maintain in this codebase?

@kennethbruskiewicz kennethbruskiewicz merged commit 6b7f47c into master Nov 9, 2023
1 check passed
Copy link
Collaborator

@kennethbruskiewicz kennethbruskiewicz left a comment

Choose a reason for hiding this comment

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

Looks good. I'm still learning how to write code for Handsontable.

It looks like the main move here other than tagging the input element is to ensure the event handlers are registered against the Handsontable singleton, because that object manages its entire presence on the DOM. I'm assuming this is necessary more for the date picking controls to work, rather than the copy-paste functionality. Would like to learn more.

@ddooley
Copy link
Collaborator

ddooley commented Nov 9, 2023

Thanks Patrick - it works great; and thx Ken for testing. I'll ensure the other cut/paste changes in mpox rename branch dataharmonizer.js are removed.

@ddooley
Copy link
Collaborator

ddooley commented Nov 9, 2023

Hmm, there may be one lingering problem. When DH loads, do a paste into a date field. Then switch templates. I'm getting something that hopefully is easy to remedy in FlatpickrEditor.js:

Uncaught runtime errors:
×
ERROR
Cannot read properties of undefined (reading 'destroy')
TypeError: Cannot read properties of undefined (reading 'destroy')
    at DateEditor.destroyElements (webpack-internal:///../lib/editors/FlatpickrEditor.js:88:13)
...

@pkalita-lbl
Copy link
Collaborator Author

I could be wrong but there are no templates in DataHarmonizer right now that take "xsd:datetime" and "xsd:time" as ranges, only "date".

No you're right. NMDC uses datetimes though. Since this fix is in the FlatpickrEditor base class it should apply to the date, datetime, and time cell editors.

is there anything about how the HOT editors currently work which should also apply to the other editors we maintain in this codebase?

Not that I'm aware of!

When DH loads, do a paste into a date field. Then switch templates. I'm getting something that hopefully is easy to remedy in FlatpickrEditor.js

Oops, yeah, I see. Sounds like a timing issue between when the HT instance is recreated for the new template and some asynchronous event callbacks get handled on the old editor. Should be straightforward to fix. I'll get a PR in shortly.

@ddooley ddooley deleted the fix-date-editor-keyboard-copy branch November 10, 2023 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants