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: Support inverted horizontal scrolling for right-to-left locales #787

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

cachgill
Copy link
Contributor

@cachgill cachgill commented Aug 7, 2024

This change adds an option for consumers who need to switch between left-to-right and right-to-left language locales.

With this change, scrolling left-to-right:
https://github.com/user-attachments/assets/93892f15-a5fe-4cdc-8ccf-45c316a966f0

Without this change, scrolling right-to-left:
https://github.com/user-attachments/assets/47aa1cae-de3d-4946-9fd8-29fee19fe560

With this change, scrolling right-to-left:
https://github.com/user-attachments/assets/ab87e0f7-0679-4669-986d-69188d572f7f

offset = element[instance.options.horizontal ? 'scrollLeft' : 'scrollTop']
const { horizontal, isRtl } = instance.options
offset = horizontal
? element['scrollLeft'] * (isRtl && -1 || 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep it simple

Suggested change
? element['scrollLeft'] * (isRtl && -1 || 1)
? element['scrollLeft'] * (isRtl ? -1 : 1)

Did you test it on all modern browsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are used to adhering to ESlint's rules regarding ternaries: https://eslint.org/docs/latest/rules/no-nested-ternary

The scenario in the included videos was tested in Safari, Chrome, Firefox, Brave, Opera, and Edge.

Copy link

nx-cloud bot commented Aug 14, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit c257810. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Copy link

pkg-pr-new bot commented Aug 14, 2024

commit: c257810

@tanstack/lit-virtual

pnpm add https://pkg.pr.new/@tanstack/lit-virtual@787

@tanstack/react-virtual

pnpm add https://pkg.pr.new/@tanstack/react-virtual@787

@tanstack/solid-virtual

pnpm add https://pkg.pr.new/@tanstack/solid-virtual@787

@tanstack/svelte-virtual

pnpm add https://pkg.pr.new/@tanstack/svelte-virtual@787

@tanstack/virtual-core

pnpm add https://pkg.pr.new/@tanstack/virtual-core@787

@tanstack/vue-virtual

pnpm add https://pkg.pr.new/@tanstack/vue-virtual@787

Open in Stackblitz

More templates

@piecyk piecyk merged commit 2f5821a into TanStack:main Aug 14, 2024
5 checks passed
@piecyk
Copy link
Collaborator

piecyk commented Aug 14, 2024

Thanks @cachgill 🙌

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.

2 participants