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: expose scrollSnaps selectedIndex scrollTo #1052

Merged
merged 1 commit into from
Apr 27, 2024
Merged

feat: expose scrollSnaps selectedIndex scrollTo #1052

merged 1 commit into from
Apr 27, 2024

Conversation

shyakadavis
Copy link
Contributor

@shyakadavis shyakadavis commented Apr 27, 2024

Had briefly touched on this is in the server, bringing it here for your consideration;

This P.R lays the groundwork necessary to create a dots-scroll-to feature like in the video. From now on, one need only reach into the context and grab these stores+function and style however they wish.

const { scrollTo, scrollSnaps, selectedIndex } = getEmblaContext('<Carousel.Dots/>');

It's pretty easy to set up yourself, but having it come with the carousel component saves some time. 🙂
(And I don't think this introduction violates/deviates from the original, wouldn't you agree? - at least components-wise)


Screen.Recording.2024-04-27.at.10.12.31.mov

Before submitting the PR, please make sure you do the following

  • If your PR isn't addressing a small fix (like a typo), it references an issue where it is discussed ahead of time and assigned to you. In many cases, features are absent for a reason.
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Format & lint the code with pnpm format and pnpm lint

Copy link

changeset-bot bot commented Apr 27, 2024

⚠️ No Changeset found

Latest commit: 6bb1da7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Apr 27, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
shadcn-svelte ✅ Ready (View Log) Visit Preview 6bb1da7

Copy link
Owner

@huntabyte huntabyte left a comment

Choose a reason for hiding this comment

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

Thank you!

@huntabyte huntabyte changed the title feat: expose scrollSnaps, selectedIndex, & scrollTo in embla context feat: expose scrollSnaps selectedIndex scrollTo Apr 27, 2024
@huntabyte huntabyte merged commit 9125246 into huntabyte:main Apr 27, 2024
4 checks passed
@shyakadavis shyakadavis deleted the feat/extend-embla-carousel-context branch April 27, 2024 23:06
@danpii
Copy link

danpii commented Sep 24, 2024

Hi @shyakadavis,
I am trying to create a <Carousel.Dots/> component but can't get my head around on how to implement it in a clean way to also update the active dot, when the prev or next buttons are clicked.
I guess I need to use selectedScrollSnap() on the API?

Do you maybe have an example?

@shyakadavis
Copy link
Contributor Author

Hello, @danpii

Sure. Here is my lib/components/ui/carousel/carousel-dots.svelte file. Just make sure to remove the snake_case variable names and replace with the ones from shad. Let me know if it works out.

<script lang="ts">
	import type { HTMLAttributes } from 'svelte/elements';
	import { crossfade } from 'svelte/transition';
	import { cubicInOut } from 'svelte/easing';
	import { Button } from '../button';
	import { getEmblaContext } from './context';
	import { cn } from '$lib/utils';

	type $$Props = HTMLAttributes<HTMLDivElement>;

	let className: $$Props['class'] = undefined;
	export { className as class };

	const { handleKeyDown, scroll_to, scroll_snaps, selected_index } =
		getEmblaContext('<Carousel.Dots/>');

	const [send, receive] = crossfade({
		duration: 250,
		easing: cubicInOut
	});
</script>

<div class="absolute bottom-32 right-28 hidden h-9 items-center gap-2 md:flex">
	{#each $scroll_snaps as _, i}
		{@const is_active = $selected_index === i}
		<Button
			size="icon"
			on:click={() => scroll_to(i)}
			on:keydown={handleKeyDown}
			variant="accent"
			class={cn(
				'relative size-3 touch-manipulation rounded-full bg-border/10 transition-colors ease-linear hover:bg-border/30',
				className
			)}
		>
			{#if is_active}
				<div
					class={cn('absolute inset-0 rounded-full bg-border/60')}
					in:send={{ key: 'active_slide' }}
					out:receive={{ key: 'active_slide' }}
				/>
			{/if}
		</Button>
	{/each}
</div>

@danpii
Copy link

danpii commented Sep 25, 2024

Thanks for your quick reply @shyakadavis!

Looks good and the scrollTo is working correctly. The only thing that's not working yet is the active bullet.
It seems {@const is_active = $selectedIndex === i} is not updated correctly. The is_active div is always shown inside the first bullet button.
Any advice? Do I need to make an adjustment on the parent component as well?

@shyakadavis
Copy link
Contributor Author

Hmm... hard for me to summarise without looking at some code, but my guess is that the selectedIndex store isn't being updated whenever you select an option.

Inside carousel.svelte make sure you're doing just that.

//
++ const selectedIndex = writable(0);
//
function onSelect(api: CarouselAPI) {
		if (!api) return;
		canScrollPrev.set(api.canScrollPrev());
		canScrollNext.set(api.canScrollNext());
++		selectedIndex.set(api.selectedScrollSnap());
	}
//
$: if (api) {
++		onSelect(api);
++		api.on('select', onSelect);
++		api.on('reInit', onSelect);
	}
//

If this is the issue, I wonder if I forgot to add this store in the P.R. Please confirm if it is.

@danpii
Copy link

danpii commented Sep 25, 2024

Thanks again, really appreciated! It works now!

In this PR (#1052 )you added:
const selectedIndexStore = writable(0);

This part was already in the code (if I see it correctly):

$: if (api) {
	onSelect(api);
	api.on("select", onSelect);
	api.on("reInit", onSelect);
}

So all I had to do was:

function onSelect(api: CarouselAPI) {
	if (!api) return;
	canScrollPrev.set(api.canScrollPrev());
	canScrollNext.set(api.canScrollNext());
+	selectedIndexStore.set(api.selectedScrollSnap());
}

@shyakadavis
Copy link
Contributor Author

Snap. Thought I had covered this.

Thank you for confirming. If I get time soon I'll open a follow-up P.R. (Or if you have the time right now, you could help.)

Regardless, thank you.

@danpii
Copy link

danpii commented Sep 25, 2024

I can try as in the course of this week

@shyakadavis
Copy link
Contributor Author

#1289

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.

3 participants