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 property hide-page-indicator #3268

Merged
merged 3 commits into from May 31, 2021

Conversation

gmkv
Copy link
Contributor

@gmkv gmkv commented May 18, 2021

The page indicator can now be hidden by setting the property
hide-page-indicator on the ui5-carousel element.

  • the arrows are always visible when arrowsPlacement is set to Navigation
  • fixed typos
  • changed height reduction of the carousel viewport to 1rem

Closes: #3158

@gmkv gmkv requested review from a team, ilhan007, vladitasev, pskelin and fifoosid May 18, 2021 11:43
@gmkv gmkv closed this May 18, 2021
@gmkv gmkv reopened this May 18, 2021
}

_onmouseover() {
this._visibleNavigationArrows = true;
if (this.arrowsPlacement === CarouselArrowsPlacement.Content) {
this._visibleNavigationArrows = true;
Copy link
Member

@ilhan007 ilhan007 May 18, 2021

Choose a reason for hiding this comment

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

there is a property "hideNavigationArrows", is it considered here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was deleted by #3228

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The check for hideNavigationArrows happens during rendering. At this point in execution, it is established that arrowsPlacement will not change.

@ilhan007
Copy link
Member

You stated that "the arrows are always visible when arrowsPlacement is set to Navigation".
Is it the way it should work, should the hideNavigationArrows be considered?

@gmkv
Copy link
Contributor Author

gmkv commented May 18, 2021

You stated that "the arrows are always visible when arrowsPlacement is set to Navigation".
Is it the way it should work, should the hideNavigationArrows be considered?

The property hideNavigationArrows will still hide the arrows, regardless of where they are set to show. You can't hide the arrows in OpenUI5.

@gmkv gmkv closed this May 20, 2021
@gmkv gmkv reopened this May 20, 2021
gmkv added 3 commits May 20, 2021 16:42
The page indicator can now be hidden by setting the property
hide-page-indicator on the ui5-carousel element.

- the arrows are always visible when `arrowsPlacement` is set to `Navigation`
- fixed typos
- changed height reduction of the carousel viewport to 1rem
@gmkv gmkv force-pushed the carousel-hide-page-indicator branch from 0f04d8e to 837b315 Compare May 20, 2021 13:42
@gmkv gmkv requested a review from a team May 21, 2021 14:00
@ilhan007 ilhan007 merged commit e13a4c9 into master May 31, 2021
@ilhan007 ilhan007 deleted the carousel-hide-page-indicator branch May 31, 2021 10:34
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.

Carousel: Need property support to hide the pagination
3 participants