feat(module:carousel): support nzArrows - #9355
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9355 +/- ##
=======================================
Coverage 92.27% 92.27%
=======================================
Files 560 560
Lines 19743 19744 +1
Branches 3029 3029
=======================================
+ Hits 18217 18218 +1
Misses 1194 1194
Partials 332 332 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -17,22 +17,24 @@ | |||
|
|
|||
| .slick-prev { | |||
| .@{carousel-prefix-cls}-rtl & { | |||
There was a problem hiding this comment.
I found that when the layout direction is changed to RTL, the prev and next arrows do not adjust their positions. Maybe we should not use .@{carousel-prefix-cls}-rtl & {}, but use the following method instead, WDYT?
&-rlt {
// ...
.slick-prev {
// ...
&::after {
// ...
}
}
}
There was a problem hiding this comment.
@WwwHhhYran after checking the documentation of antd, the behavior we have currently without apply suggest change is the same as the official documentation
I suggest to keep it like that to have the same behavior as the officical documentation
WDYT ?
here is a print screen of the official ant design documentation:
There was a problem hiding this comment.
@Nicoss54 Yes, I have also checked the antd documentation, but I think it's counterintuitive. I'm not even sure if it's a design specification or a bug, so I will try to confirm this with the antd community
There was a problem hiding this comment.
I agree :) but even if it’s rtl arrow are well designed but I thinks it’s the behavior that changed. Previous become next and next become previous ?
WDYT?
There was a problem hiding this comment.
I agree :) but even if it’s rtl arrow are well designed but I thinks it’s the behavior that changed. Previous become next and next become previous ?
WDYT?
I think we use CSS to move the position of arrows and rotate them just to adapt to the layout direction, but the behavior of pre/next arrow hasn't changed: Clicking on the pre arrow will still switch to the previous content, and similarly, clicking on the next arrow will still switch to the next content.
I'm not sure if my statement is clear, so I'll use an example to illustrate:
When we switch direction to RTL, the next arrow is now on the right and pointing to the right, and after adjustment, the next arrow will be on the left and pointing to the left (Just like the suggested change below #9355 (comment)). At this point, clicking on the next arrow will switch to the second page, and the dots below will also highlight the second one (located to the left of the first dot). This behavior is the same as when the direction is ltr.
There was a problem hiding this comment.
You’re agree. Let’s do that. When you have the feedback of the community I will update the code :)
are you agree with this ? :)
There was a problem hiding this comment.
@Nicoss54 I'm sorry, I was on vacation a few days ago and didn't reply promptly. The antd community has confirmed that this is a bug ant-design/ant-design#54781
There was a problem hiding this comment.
Hope you have some rest :) so let’s fix it. I will finished the dev today :)
There was a problem hiding this comment.
@WwwHhhYran it's done, arrow are reverse correctly in rtl mode :)
ae2e775 to
89b605d
Compare
c3b2d09 to
5f1edbe
Compare
|
@Laffery your changes requested are done :) |
3 participants
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Carousel component now support nzArrows input which let the user to display arrow for swipping
Does this PR introduce a breaking change?
Other information
Related: #8826