-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feature/meditor 844 resolve sluggish text input #71
Feature/meditor 844 resolve sluggish text input #71
Conversation
async function handleSave(largeModel: boolean) { | ||
if (largeModel) { | ||
// This hack is in place to handle RJSF's ajv8 validator performance issues. | ||
await wait(3000) |
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.
The wait time could be configurable in the schema.
@@ -189,9 +198,24 @@ const FormActions = ({ | |||
<Button | |||
className={styles.button} | |||
variant="secondary" | |||
onClick={handleSave} | |||
onClick={async () => { | |||
setIsSaving(true) |
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.
I should use a setTimeout
here to remove the spinner in case the document fails to validate and save.
import validator from '@rjsf/validator-ajv8' | ||
import type { FormValidation } from '@rjsf/utils' | ||
|
||
class LargeModelForm< |
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.
Hacky shenanigans...move the change
event to be triggered on blur
. This form is only used if the accompanying model is marked as largeModel
.
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.
Related to a Tony's PR. See test file.
@@ -200,6 +200,11 @@ describe('Documents', () => { | |||
//* Normalize by deleting properties that will always have a time-based fresh value. | |||
delete firstInsertedAlert._id | |||
delete firstInsertedAlert['x-meditor'].modifiedOn | |||
//* Since change e3876fb4f226e6f0e84e24095e295dfea687089b we set the `modifiedOn` property of a document's root state. |
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.
Since the root document state now sets modifiedOn
dynamically at creation time, this property isn't a good candidate for a snapshot.
…feature/MEDITOR-844-resolve-sluggish-text-input
…ssues with large schemas
async function handleSave(largeModel: boolean) { | ||
if (largeModel) { | ||
// This hack is in place to handle RJSF's ajv8 validator performance issues. | ||
await wait(500) |
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.
@benforshey save performance isn't as noticeable after upgrading AJV, reducing this save delay
> | ||
Save | ||
{isSaving && ( |
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.
Love the addition of the spinner!
@@ -82,7 +82,6 @@ export default function ConcatenatedWidget(props: WidgetProps) { | |||
// update the concatenated value anytime this field blurs or changes | |||
el.onblur = () => updateConcatenatedFieldValueFromFields(fields) | |||
el.onchange = () => updateConcatenatedFieldValueFromFields(fields) | |||
el.onkeyup = () => updateConcatenatedFieldValueFromFields(fields) |
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.
This was causing a small, but perceptible lag when dealing with concatenated fields. I think updating the concatenated field on blur and change is good enough
@@ -17,7 +17,7 @@ | |||
"@json2csv/node": "^7.0.1", | |||
"@rjsf/core": "^5.16.1", | |||
"@rjsf/utils": "^5.16.1", | |||
"@rjsf/validator-ajv8": "^5.16.1", | |||
"@rjsf/validator-ajv8": "^6.0.0-alpha.0", |
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.
Typically running alpha versions is not a good idea...however this resolves a major performance issues with RJSF v5
No description provided.