Skip to content
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

fix(ui5-calendar): switch to two column layout on Islamic or Persian secondary calendar type #8453

Merged
merged 10 commits into from
May 10, 2024

Conversation

hinzzx
Copy link
Contributor

@hinzzx hinzzx commented Mar 15, 2024

Previously when secondaryCalendarType was set to Islamic or Persian, the texts of the months' names didnt had enough space in the cell to be displayed on one row, which wasnt the desired behavior.

Now, when any of those two calendar types is set as a secondaryCalendarType, when the end user is in month view of the calendar, the months are displayed in two column layout, instead of three, providing enough space for the texts to be displayed in one row.

Tested in both Cozy/Compact modes in following scenarios:


Primary/SecondaryCalendarType:

  1. Gregorian/Buddhist
  2. Gregorian/Islamic
  3. Gregorian/Japanese
  4. Gregorian/Persian
  5. Buddhist/Gregorian
  6. Buddhist/Islamic
  7. Buddhist/Japanese
  8. Buddhist/Persian
  9. Islamic/Gregorian
  10. Islamic/Buddhist
  11. Islamic/Japanese
  12. Islamic/Persian
  13. Japanese/Gregorian
  14. Japanese/Buddhist
  15. Japanese/Islamic
  16. Japanese/Persian
  17. Persian/Gregorian
  18. Persian/Buddhist
  19. Persian/Islamic
  20. Persian/Japanese

Before

image

After

image

Fixes: #7712

@unazko unazko self-requested a review March 18, 2024 08:20
Copy link
Contributor

@unazko unazko left a comment

Choose a reason for hiding this comment

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

The same issue isn't reproducible in openui5 project with Islamic secondary calendar type due to difference in the gridcell sizes. As you already mentioned the ui5-calendar is the one that has gridcell sizes not according to the visual design. The issue is reproducible with the Persian secondary calendar type on both projects. We'll need to discuss the idea of the two column month layout with the visual designers as in this case a vertical scrolling appears:
Persian
Islamic

@hinzzx hinzzx changed the title fix(ui5-monthpicker): switch to two column layout on Islamic or Persian secondary calendar type fix(ui5-calendar): switch to two column layout on Islamic or Persian secondary calendar type Mar 27, 2024
packages/main/src/MonthPicker.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@unazko unazko left a comment

Choose a reason for hiding this comment

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

All fine except the Home/End keyboard interactions.

@hinzzx hinzzx requested review from unazko and nnaydenow April 10, 2024 14:25
Copy link
Contributor

@tsanislavgatev tsanislavgatev left a comment

Choose a reason for hiding this comment

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

Two things, when changing from month to year picker, there is a moving of the header of the calendar. The arrows and the button are going lower. Also when primary and secondary calendar are the same and are one of the exceptions, the calendar is still in 2 columns

@hinzzx
Copy link
Contributor Author

hinzzx commented Apr 26, 2024

Two things, when changing from month to year picker, there is a moving of the header of the calendar.

The problem isn't introduced by the change. Outdated

...when primary and secondary calendar are the same and are one of the exceptions, the calendar is still in 2 columns

Done ✅

EDIT: I was testing different case. There is actually header jump on two-column layout, so it will be fixed within this change.

@nnaydenow
Copy link
Contributor

Two things, when changing from month to year picker, there is a moving of the header of the calendar.

The problem isn't introduced by the change.

...when primary and secondary calendar are the same and are one of the exceptions, the calendar is still in 2 columns

Done ✅

If you are not going to fix issue with current PR please reference issue where the issue could be tracked.

@hinzzx hinzzx merged commit 1b172eb into main May 10, 2024
10 checks passed
@hinzzx hinzzx deleted the sec-cal-type-mp-fix branch May 10, 2024 08:28
ilhan007 pushed a commit that referenced this pull request May 13, 2024
…secondary calendar type (#8943)

Properly switches to two column layout when Islamic or Persian secondary calendar type are used

Fixes: #8453
hinzzx added a commit that referenced this pull request May 22, 2024
…secondary calendar type (#8453)

Now, when any of the two calendar types (Persian or Islamic) is set as a secondaryCalendarType, when the end user is in month view of the calendar, the months are displayed in two column layout, instead of three, providing enough space for the texts to be displayed in one row.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Calendar]: Secondary calendar labels reach outside of control
4 participants