-
Notifications
You must be signed in to change notification settings - Fork 665
DataGrid - AI Column: Endless prompt editor loading when edit prompt and rollback changes #31848
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
DataGrid - AI Column: Endless prompt editor loading when edit prompt and rollback changes #31848
Conversation
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.
Pull request overview
This PR fixes an issue where the AI prompt editor would enter an endless loading state when a user edits a prompt and then rolls back the changes to the original value. The fix adds a check to prevent triggering the loading state and column option updates when the prompt value hasn't actually changed.
Key changes:
- Added early return in
onSubmithandler when prompt value hasn't changed from the original - Added comprehensive test coverage for the new behavior with two test cases
- Included temporary debug logging for troubleshooting
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/devextreme/js/__internal/grids/grid_core/ai_column/views/m_ai_prompt_editor_view.ts |
Added change detection logic in onSubmit to prevent unnecessary state updates and API calls when prompt hasn't changed; includes temporary debug console.log statements |
packages/devextreme/js/__internal/grids/grid_core/ai_column/views/m_ai_prompt_editor_view.test.ts |
Added two new test cases to verify that loading state is not triggered when prompt is unchanged and is properly triggered when prompt changes |
packages/devextreme/js/__internal/grids/grid_core/ai_column/views/m_ai_prompt_editor_view.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/grids/grid_core/ai_column/views/m_ai_prompt_editor_view.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/grids/grid_core/ai_column/views/m_ai_prompt_editor_view.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/grids/grid_core/ai_column/views/m_ai_prompt_editor_view.ts
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
| height: 110, | ||
| stylingMode: 'outlined', | ||
| onValueChanged: (e): void => { | ||
| if (e.value === this.prompt) { |
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 think you can write it more simply:
this.updateButtonOption(APPLY_BUTTON_INDEX, 'disabled', !e.value || e.value === this.prompt);
this.updateButtonOption(REGENERATE_DATA_BUTTON_INDEX, 'disabled', !e.value || e.value !== this.prompt);
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.
You can also move the e.value === this.prompt condition to a separate variable. For example, isValueChanged.
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.
thank you! Did it
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
No description provided.