Scheduler — Replace underscore-prefixed: Workspace group (agenda, timeline, month, indicator)#32797
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors DevExtreme Scheduler internal workspace implementations to remove underscore-prefixed members (agenda/timeline/month/indicator) and updates Scheduler QUnit tests to use the new member names.
Changes:
- Renamed several underscore-prefixed methods/properties to non-underscore equivalents across agenda/timeline/month/indicator workspaces (with updated call sites/overrides).
- Updated Scheduler agenda unit/integration tests to stub/call the renamed workspace members.
- Tightened visibility for some refactored members (e.g.,
private) in internal workspace classes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/integration.agenda.tests.js | Updates integration tests to use getRowHeight and rows instead of underscore-prefixed members. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/agenda.tests.js | Updates unit tests to call recalculateAgenda / removeEmptyRows instead of underscore-prefixed methods. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_work_space_month.ts | Renames month workspace helpers and updates binding used during table rendering. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_work_space_indicator.ts | Renames indicator API (renderIndicator, createIndicator, etc.) and updates internal usage. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_timeline_work_week.ts | Renames and re-wires date-increment override to incrementDate. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_timeline_week.ts | Renames week header/date increment hooks to non-underscore methods. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_timeline_month.ts | Renames duration calculation hook to calculateDurationInCells. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_timeline_day.ts | Renames week header rendering hook to needRenderWeekHeader. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_timeline.ts | Updates timeline workspace to call renamed indicator/week-header/date-increment hooks and adjusts related helpers. |
| packages/devextreme/js/__internal/scheduler/workspaces/m_agenda.ts | Refactors agenda workspace internal state/methods away from underscore-prefixed names and updates internal calls. |
packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/agenda.tests.js
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/workspaces/m_work_space_indicator.ts
Outdated
Show resolved
Hide resolved
…eline, month, indicator)
- m_work_space_indicator.ts: revert createIndicator to _createIndicator (used in subclass, cannot be private), remove stale comment - m_timeline.ts: update calls to _createIndicator - agenda.tests.js: update test name to match renamed method
…h createIndicator method
aleksei-semikozov
force-pushed
the
3519-workspace-group-replace-underscore-partial
branch
from
087d578 to
59bcda1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
packages/devextreme/js/__internal/scheduler/__tests__/workspace.base.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
| // Overridden in SchedulerTimelineMonth | ||
| calculateDurationInCells(timeDiff) { |
There was a problem hiding this comment.
If it's overriden, can we make it protected?
| protected override updateAllDayVisibility() { return noop(); } | ||
|
|
||
| _updateAllDayHeight() { return noop(); } | ||
| private updateAllDayHeight() { return noop(); } |
There was a problem hiding this comment.
Isn't it protected? Usually if method uses noop(), then it is overriden somewhere. If not, then this method useless
| } | ||
|
|
||
| _cleanCurrentTimeCells(): void { | ||
| cleanCurrentTimeCells(): void { |
There was a problem hiding this comment.
Looks like this method is not called anywhere. Can we remove it?
| } | ||
|
|
||
| _calculateDurationInCells(timeDiff) { | ||
| calculateDurationInCells(timeDiff) { |
There was a problem hiding this comment.
Can we mark it as protected? And this method too?:
and like overall can we put protected where needed? Because this methods were "private" before, but now they look public, even though they are not
| _getToday: any; | ||
| protected getToday?(): Date; | ||
|
|
||
| _$allDayPanel: any; |
There was a problem hiding this comment.
why not put private and protected modifier to other props in this class?
| } | ||
|
|
||
| _getCurrentTimePanelCellIndices() { | ||
| // Overridden in SchedulerTimeline |
There was a problem hiding this comment.
Can we put protected then, if it's overriden?
No description provided.