Skip to content

Fix/carousel initial page on android #3047

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Fix/carousel initial page on android #3047

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
278d02a
Fix initial page on android rtl
nitzanyiz Apr 24, 2024
0e35dec
formatting
nitzanyiz Apr 24, 2024
a1d0853
formatting
nitzanyiz Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/carousel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ class Carousel extends Component<CarouselProps, CarouselState> {
pagingEnabled={this.shouldEnablePagination()}
snapToOffsets={snapToOffsets}
contentOffset={contentOffset}
// onContentSizeChange={this.onContentSizeChange}
onContentSizeChange={this.onContentSizeChange}
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you checked it's not messing up something related to this PR ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I saw that they commented that this might not be needed in their pr. It seems to fix the starting page problem now.
Regarding the ticket the the PR you mentioned fixed. Its not reproduced after this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I used this fix and tested in on our demo app, in Android phone in Hebrew.
The index does fixed in RTL, but the initial index passed is 0 and on RTL it's starting on 6, does this PR should solve this also ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How did you reproduce this? What code did you use?

Copy link
Contributor

Choose a reason for hiding this comment

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

Run the app on Android emulator, make sure to force RTL.
In the carousel screen (in the private repo) check the INITIAL_PAGE value, the currentPage in the state is the reversed value of the INITIAL_PAGE.

onMomentumScrollEnd={this.onMomentumScrollEnd}
style={_style}
>
Expand Down