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(react-instantsearch): add Carousel layout component #6321

Merged
merged 22 commits into from
Aug 19, 2024

Conversation

shaejaz
Copy link
Contributor

@shaejaz shaejaz commented Aug 9, 2024

This adds the the Carousel layout component and enables the layoutComponent prop in Recommend widgets. React getting started example has been update to use Carousel as well.

image

https://algolia.atlassian.net/browse/FX-2819

Copy link

codesandbox-ci bot commented Aug 9, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5764eb9:

Sandbox Source
example-instantsearch-getting-started Configuration
example-react-instantsearch-getting-started Configuration
example-react-instantsearch-next-app-dir-example Configuration
example-react-instantsearch-next-routing-example Configuration
example-vue-instantsearch-getting-started Configuration

Copy link
Member

@sarahdayan sarahdayan left a comment

Choose a reason for hiding this comment

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

Almost there, a couple changes and we should be good.

I also noticed that the box shadow is being cut off in demo because of the overflow ruleset. I fixed it in the JS example, feel free to compare markup and styles to replicate it.

Capture d’écran 2024-08-09 à 15 21 25

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

🔥

@@ -90,7 +90,6 @@ export function createTrendingItemsComponent({

<Layout
classNames={cssClasses}
translations={translations}
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean the "previous" and "next" labels aren't customisable, or is that done another way now?

Copy link
Member

@sarahdayan sarahdayan Aug 12, 2024

Choose a reason for hiding this comment

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

The translations we passed were the ones of the widget, and it didn't make sense to do so. Layouts have their own translations which they receive as a prop, but there's no point in passing widget translations.

This was carried over from the previous design from Recommend v1 where slider translations were all at the widget level, but the design we created makes them independent. This is also the reason why we should most probably deprecate the sliderLabel translation.

@shaejaz shaejaz requested review from sarahdayan and Haroenv August 14, 2024 09:52
Copy link
Member

@dhayab dhayab left a comment

Choose a reason for hiding this comment

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

👏

@shaejaz shaejaz force-pushed the feat/react-is-carousel branch from bc219d7 to 0f2d896 Compare August 19, 2024 10:08
@shaejaz shaejaz requested a review from Haroenv August 19, 2024 15:24
@shaejaz shaejaz merged commit 41b0c96 into master Aug 19, 2024
13 checks passed
@shaejaz shaejaz deleted the feat/react-is-carousel branch August 19, 2024 15:39
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.

4 participants