Skip to content

Scheduler: Appointment Form - fix paddings#32865

Open
sjbur wants to merge 20 commits intoDevExpress:26_1from
sjbur:issue-3609_26_1
Open

Scheduler: Appointment Form - fix paddings#32865
sjbur wants to merge 20 commits intoDevExpress:26_1from
sjbur:issue-3609_26_1

Conversation

@sjbur
Copy link
Contributor

@sjbur sjbur commented Mar 11, 2026

No description provided.

@sjbur sjbur self-assigned this Mar 11, 2026
@sjbur sjbur added the 26_1 label Mar 11, 2026
@sjbur sjbur closed this Mar 11, 2026
@sjbur sjbur reopened this Mar 11, 2026
@sjbur sjbur marked this pull request as ready for review March 17, 2026 12:06
@sjbur sjbur requested a review from a team as a code owner March 17, 2026 12:06
.dx-item-content:has(.dx-texteditor-with-floating-label):not(:has(.dx-scheduler-form-all-day-switch)) .dx-scheduler-form-icon.dx-field-item,
.dx-item-content:has(.dx-texteditor-with-label):not(:has(.dx-scheduler-form-all-day-switch)) .dx-scheduler-form-icon.dx-field-item {
height: $generic-scheduler-appointment-popup-icon-static-label-container-height;
padding-top: $generic-scheduler-appointment-popup-icon-static-label-padding-top;
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have styles for the icon in generic theme for static label mode:

margin-top: $generic-scheduler-appointment-popup-icon-inner-label-margin-top;

just need to set the size here:

$generic-scheduler-appointment-popup-icon-inner-label-margin-top: null !default;

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need these styles in this case?

});
});

test.meta({ browserSize: [1500, 1500] })('appointment form with labelMode=static', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

pls add a test for a case when main group doesn't have icons. Since we made icons invisible when they are disabled, some paddings will not appear, so the main group visual is changed.

Comment on lines -229 to -231
const iconsShowMode = this.getIconsShowMode();
const showMainGroupIcons = ['main', 'both'].includes(iconsShowMode);
const showRecurrenceGroupIcons = ['recurrence', 'both'].includes(iconsShowMode);
Copy link
Contributor

@Tucchhaa Tucchhaa Mar 17, 2026

Choose a reason for hiding this comment

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

I understand your solution: propagate showIcon parameter through each function and duplicate logic of selecting colSpan:
colSpan: showIcon ? 1 : 2

While this works, the problem is that the logic is duplicated and we can easily forget to update some item's colSpan.

Here's the alternative approach by using setStylingModeToEditors:

  private setStylingModeToEditors(item: FormItem, showIcon: boolean): void {
    const itemClasses = (item.cssClass ?? '').split(' ');
    const isIconItem = itemClasses.includes(CLASSES.formIcon);

    if (isIconItem) {
      return;
    }

    if (item.itemType === 'simple') {
      // ...
      return;
    }

    if (item.itemType === 'group') {
      const groupItem = item as GroupItem;
      const colCount = showIcon ? 2 : 1;
      groupItem.colCount = colCount;
      groupItem.colCountByScreen = { xs: colCount };
      groupItem.items?.forEach((child) => {
        this.setStylingModeToEditors(child, showIcon);
      });
    }
  }

This way we can remove:

  • showIcon parameter propagation
  • duplication of showIcon ? 1 : 2 logic
  • groupItems' colCount duplicated option

What do you think?

P.S. the codesnippet is for ref only, I haven't tested it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants