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

feat(ui5-carousel): Implement F7 keyboard functionality #3559

Merged
merged 12 commits into from
Aug 5, 2021
Merged

Conversation

TeodorTaushanov
Copy link
Member

@TeodorTaushanov TeodorTaushanov commented Jul 28, 2021

Part of #3092

@TeodorTaushanov TeodorTaushanov requested review from fifoosid, vladitasev, ilhan007 and a team July 29, 2021 14:51
Copy link
Contributor

@georgimkv georgimkv left a comment

Choose a reason for hiding this comment

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

On http://localhost:8080/test-resources/pages/Carousel.html
If the focus is on a list item within a card, pressing F7 twice will not restore the focus to the same item.
_onfocusin in the Carousel does not get called every time - in particular, when the focus moves from the card header to the list in the card.

Can you also write a test?

@TeodorTaushanov
Copy link
Member Author

On http://localhost:8080/test-resources/pages/Carousel.html
If the focus is on a list item within a card, pressing F7 twice will not restore the focus to the same item.
_onfocusin in the Carousel does not get called every time - in particular, when the focus moves from the card header to the list in the card.

Can you also write a test?

Tests are added.

The ui5-list stops propagation of the "focusin" event. For that F7 keyboard navigation doesn't work for it. This should be fixed in the ui5-list.

georgimkv
georgimkv previously approved these changes Jul 30, 2021
ilhan007
ilhan007 previously approved these changes Jul 30, 2021
@CLAassistant
Copy link

CLAassistant commented Jul 31, 2021

CLA assistant check
All committers have signed the CLA.

@ilhan007 ilhan007 dismissed stale reviews from georgimkv and themself via 29f6a1b August 3, 2021 11:39
@ilhan007
Copy link
Member

ilhan007 commented Aug 5, 2021

+1, someone from @SAP/ui5-webcomponents-topic-rd should also review the change

@ilhan007 ilhan007 merged commit df0ace8 into master Aug 5, 2021
@ilhan007 ilhan007 deleted the CarouselF7 branch August 5, 2021 09:19
@ilhan007
Copy link
Member

ilhan007 commented Aug 5, 2021

If this was the last change related to the Keyboard Handling epic, you can close the issue
#3092

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