Skip to content

Conversation

@GDamyanov
Copy link
Contributor

@GDamyanov GDamyanov commented Nov 24, 2025

  • Calendar header navigation buttons are now focusable via keyboard (tabindex set).
  • Each header button has a descriptive title attribute for improved accessibility and user guidance.

@ui5-webcomponents-bot
Copy link
Collaborator

ui5-webcomponents-bot commented Nov 24, 2025

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview November 24, 2025 15:10 Inactive
@GDamyanov GDamyanov changed the title feat(ui5-calendar): enhance component accessibility feat(ui5-calendar): improve header and day picker accessibility and focus behavior Nov 24, 2025
@GDamyanov GDamyanov self-assigned this Nov 24, 2025
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview November 24, 2025 15:45 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview November 24, 2025 19:14 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview November 25, 2025 12:41 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview November 25, 2025 13:24 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 8, 2025 17:27 Inactive
@GDamyanov GDamyanov marked this pull request as ready for review December 8, 2025 17:33
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 8, 2025 17:38 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 8, 2025 18:30 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 9, 2025 09:53 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 9, 2025 10:15 Inactive
@GDamyanov GDamyanov requested a review from unazko December 10, 2025 11:20
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 10, 2025 11:31 Inactive
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.

Overall the code is clean.
I've found the following behavioural issue:

  • Right mouse click triggers the previous/next button's action

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 16, 2025 10:28 Inactive
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 code looks good in my perspective.

There is one last issue:

  • In the year range view the next/previous button tooltips should refer to year ranges instead of months.

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 17, 2025 07:21 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 17, 2025 09:01 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 17, 2025 09:18 Inactive
@GDamyanov GDamyanov requested a review from unazko December 17, 2025 13:02
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 17, 2025 13:08 Inactive
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 code looks good to me.
Tested with the accessibility manually with screen reader and AMP tool.
Everything works fine.

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 17, 2025 13:45 Inactive

cy.focused().realPress(["Shift", "F4"]);

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

.find("[tabindex='0']")
.should("have.focus");

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

cy.get("@selectedDays").each(($day, index) => {
cy.wrap($day).should("have.attr", "aria-label");

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd


describe("Day Picker Tests", () => {
it.skip("Select day with Space", () => {
it.skip("Select day with Space", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

it.skip("Select day with Space", () => {
cy.mount(<Calendar id="calendar1"></Calendar>);

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

expect(selectedDate).to.eq(expectedDate);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd


cy.mount(<Calendar id="calendar1"></Calendar>);

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

it("Day names are correctly displayed", () => {
cy.mount(<Calendar id="calendar1"></Calendar>);

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

const todayFromTimestamp = new Date(timestamp * 1000);
const actualToday = new Date();

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

_pickersMode: `${CalendarPickersMode}` = "DAY_MONTH_YEAR";

@property({ type: Boolean, noAttribute: true })
_isOpenedFromPopover?: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55127dd

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 17, 2025 14:05 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview December 18, 2025 08:41 Inactive
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.

5 participants