Skip to content

Scheduler: Appointment Form - fix paddings#32865

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

Scheduler: Appointment Form - fix paddings#32865
sjbur wants to merge 37 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case we can avoid those styles. Rewrote them

});
});

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Will write them soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test is added

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the solution based on your suggestion

Copy link
Contributor

Choose a reason for hiding this comment

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

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

    if (isIconItem) {
      item.visible = showIcon;
      return;
    }

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

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants