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: ✨sd-carousel #324

Merged
merged 80 commits into from
Oct 10, 2023
Merged

feat: ✨sd-carousel #324

merged 80 commits into from
Oct 10, 2023

Conversation

Vahid1919
Copy link
Contributor

@Vahid1919 Vahid1919 commented Aug 8, 2023

Description:

This PR includes the work for sd-carousel as well as sd-carousel item.

Related to #239

Definition of Reviewable:

PR notes: Irrelevant elements should be removed.

  • Documentation is created/updated
  • Migration Guide is created/updated
  • E2E tests (features, a11y, bug fixes) are created/updated
  • Stories (features, a11y) are created/updated
  • relevant tickets are linked

@Vahid1919 Vahid1919 requested a review from yoezlem August 8, 2023 13:02
@Vahid1919 Vahid1919 self-assigned this Aug 8, 2023
@Vahid1919 Vahid1919 linked an issue Aug 9, 2023 that may be closed by this pull request
9 tasks
@Vahid1919 Vahid1919 changed the title DRAFT - feat: ✨sd-carousel feat: ✨sd-carousel Aug 10, 2023
@mariohamann mariohamann removed their assignment Oct 4, 2023
@@ -137,6 +137,28 @@ export const SlidesPerPage = {
}
};

/**
* Use `slides-per-move` to set how many slides the carousel advances when scrolling. Useful when specifying a `slides-per-page`
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens when slides-per-page is set to 1 and slides-per-move is set to 2? maybe you can explain the combination a bit more detailed in terms of what to expect => makes it easier to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karlbaumhauer I added some explanation there, but I thought it was better to show how it works so I expanded on it's story to show that specific example.

packages/components/src/components/carousel/carousel.ts Outdated Show resolved Hide resolved
@Vahid1919 Vahid1919 assigned Vahid1919 and unassigned MarcMatthiae Oct 5, 2023
@Vahid1919
Copy link
Contributor Author

@karlbaumhauer @mariohamann
I have updated the documentation for SlidesPerMove detailing how it should be used and how it should not. Please Review :)

@karlbaumhauer
Copy link
Contributor

@Vahid1919 pls change the version in the docs as we already are at v1.17.0 and the sd-carousel will be stable at v1.18.0 afterwards.

@karlbaumhauer karlbaumhauer removed their assignment Oct 6, 2023
@Vahid1919 Vahid1919 merged commit d1a8d0e into main Oct 10, 2023
9 checks passed
@Vahid1919 Vahid1919 deleted the feat/sd-carousel branch October 10, 2023 14:09
karlbaumhauer pushed a commit that referenced this pull request Oct 10, 2023
# [@solid-design-system/components-v1.18.0](components/1.17.0...components/1.18.0) (2023-10-10)

### Features

* ✨sd-carousel ([#324](#324)) ([d1a8d0e](d1a8d0e))
yoezlem pushed a commit that referenced this pull request Oct 25, 2023
yoezlem pushed a commit that referenced this pull request Oct 25, 2023
# [@solid-design-system/components-v1.18.0](components/1.17.0...components/1.18.0) (2023-10-10)

### Features

* ✨sd-carousel ([#324](#324)) ([d1a8d0e](d1a8d0e))
@yoezlem yoezlem added the CL-migration All components which need to be migrated from Component Library label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-migration All components which need to be migrated from Component Library
Projects
Development

Successfully merging this pull request may close these issues.

feat: ✨ add sd-carousel
9 participants