Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

feat(hooks): implement SortBy component #3373

Merged
merged 12 commits into from
Mar 4, 2022
Merged

Conversation

sarahdayan
Copy link
Member

Summary

This introduces the SortBy DOM widget.

Ticket: FX-1164

Results

  • New SortBy widget
  • DOM tests for both UI component and widget
  • Replaced SortBy with the widget in example

@sarahdayan sarahdayan requested review from Haroenv and dhayab March 2, 2022 16:32
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 2, 2022

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 b3840aa:

Sandbox Source
react-instantsearch-app Configuration
routing-basic Configuration
hooks-example Configuration

@netlify
Copy link

netlify bot commented Mar 2, 2022

✔️ Deploy Preview for react-instantsearch ready!

🔨 Explore the source changes: b3840aa

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-instantsearch/deploys/6221f6e213c8260007dac221

😎 Browse the preview: https://deploy-preview-3373--react-instantsearch.netlify.app

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.

👍

@Haroenv
Copy link
Contributor

Haroenv commented Mar 2, 2022

looks like something with the types loses specificity in the index.test.tsx, maybe try putting the component instantiation inside the switch instead of just the props?

@sarahdayan
Copy link
Member Author

@Haroenv I'm afraid that's a little too dynamic for TypeScript, and we'll need to either maintain code to make it understand the mapping better (defeating the purpose of importing all widgets from the barrel) or just disable type checking there.

Test will fail if we pass wrong parameters anyway so I'd say, let's @ts-ignore here. wdyt?

@Haroenv
Copy link
Contributor

Haroenv commented Mar 3, 2022

I fixed it in 1e1ef03 @sarahdayan, with the same cast still needed (as Object.entries isn't type-safe for some dumb reason microsoft/TypeScript#35101)

@sarahdayan sarahdayan requested a review from Haroenv March 3, 2022 17:06
@sarahdayan sarahdayan requested a review from Haroenv March 4, 2022 11:24
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.

todo bem

@sarahdayan sarahdayan merged commit 39767bb into master Mar 4, 2022
@sarahdayan sarahdayan deleted the feat/hooks-dom-sortby branch March 4, 2022 16:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants