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): add navigate event #1454

Merged
merged 11 commits into from
Apr 23, 2020
Merged

feat(ui5-carousel): add navigate event #1454

merged 11 commits into from
Apr 23, 2020

Conversation

ilhan007
Copy link
Member

@ilhan007 ilhan007 commented Apr 9, 2020

The event is fired whenever the selectedIndex changes due to user interaction - when the user clicks on the navigation arrows or while resizing, based on the "items-per-page-s/m/l" properties.

Fixes: #1449

@ilhan007 ilhan007 changed the title feat(ui5-carousel): add selectedIndexChange event feat(ui5-carousel): add selectedPageChange event Apr 9, 2020
Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

There is also the possibility of pages changing due to the user resizing the carousel or the developer removing items from the carousel.

Overall I think pages are unreliable and it would be better to start working with items. I have a change for this, but incomplete. Let's wait until I've merged it and then we can implement the events too

@ilhan007 ilhan007 changed the title feat(ui5-carousel): add selectedPageChange event feat(ui5-carousel): add navigate event Apr 10, 2020
@@ -24,13 +24,15 @@
{{#if arrows.content}}
<div class="ui5-carousel-navigation-arrows">
<ui5-button
arrow-back
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 need these?

Copy link
Member Author

Choose a reason for hiding this comment

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

it is used in the test to retrieve the backward arrow in more explicit way, but I guess ui5-carousel-navigation-arrows[0] will work as well, I can change it if you want

vladitasev
vladitasev previously approved these changes Apr 23, 2020
@ilhan007 ilhan007 merged commit c55bcdc into master Apr 23, 2020
@ilhan007 ilhan007 deleted the feat-carousel-event branch April 23, 2020 13:06
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.

ui5-carousel: add event for selected page changed
2 participants