-
Notifications
You must be signed in to change notification settings - Fork 658
Scheduler - Update Templates Demo for jQuery #31605
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
Scheduler - Update Templates Demo for jQuery #31605
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 refactors the scheduler appointment popup functionality and updates a jQuery demo. The main changes include reordering method calls in the popup initialization, renaming editor name constants for consistency, and completely refactoring the demo's form customization with improved UI.
- Refactored appointment popup form initialization to improve timing and remove redundant toolbar updates
- Updated editor name constants to have an "Editor" suffix for better clarity and consistency
- Redesigned the jQuery Templates demo with a modernized form layout and visual styling
Reviewed Changes
Copilot reviewed 5 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| m_popup.ts | Removed premature updateToolbarForMainGroup() call and moved showMainGroup(false) to after form data is set |
| m_form.ts | Renamed editor name constants to include "Editor" suffix (e.g., startDate → startDateEditor) |
| jQuery/index.js | Complete refactor of form customization with custom template, improved event handling, and modern UI components |
| jQuery/styles.css | Updated styles for the modernized demo with new layout classes and visual improvements |
| jQuery/data.js | Changed movie color palette to softer pastel colors |
packages/devextreme/js/__internal/scheduler/appointment_popup/m_popup.ts
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 5 out of 12 changed files in this pull request and generated 1 comment.
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 5 out of 12 changed files in this pull request and generated 5 comments.
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 5 out of 12 changed files in this pull request and generated 3 comments.
Signed-off-by: Eldar Iusupzhanov <[email protected]>
| if (canceled) { | ||
| e.cancel = true; | ||
| } else { | ||
| this.updateToolbarForMainGroup(); |
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.
We are really should remove this line?
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.
Yep, that's a duplicate invocation of updateToolbarForMainGroup. Because already have this callstack:
_onShowing ->
_updateForm ->
showMainGroup ->
updateToolbarForMainGroup
Sorry for the unrelated change in the PR, but I have noticed it and was afraid that I would forgot about that.
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 5 out of 12 changed files in this pull request and generated 1 comment.
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 5 out of 12 changed files in this pull request and generated 5 comments.
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 5 out of 12 changed files in this pull request and generated 1 comment.
No description provided.