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.

Sign up for GitHub

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

Jump to bottom

feat(quantic): added support for custom options in the quanticSort component #4101

Merged
merged 45 commits into from
Aug 16, 2024
Merged

feat(quantic): added support for custom options in the quanticSort component #4101

merged 45 commits into from
Aug 16, 2024

Conversation

SimonMilord
Copy link
Contributor

@SimonMilord SimonMilord commented Jun 17, 2024 •
edited
Loading

SFINT-5491

IN THIS PR:

  • Transformed the quanticSort component to accept slots for custom sort options. If no custom options are passed via slots, the current logic remains and users can still sort by: RELEVANCE, OLDEST, NEWEST
  • Created the quanticSortOption component that is used as slot for custom sorting options.
  • Made sure the state is linked between quanticSort and quanticRefineModalContent
  • Added E2E tests in cypress to cover this new feature

DEMO - DEFAULT CASE (when no quanticSortOption are passed as slots)

Screen.Recording.2024-06-18.at.1.19.52.PM.mov

DEMO - CUSTOM CASE (when quanticSortOption are passed as slots)

Screen.Recording.2024-06-18.at.1.27.13.PM.mov

Bueno validation example:

When passing for example the custom option :

{ label: 'DATE ASC', value: undefined, criterion: { by: 'date', order: 'ascending', } },

Validation fails and the component enters error state and logs the error in the console:
image

E2E TESTS: (added for both search and insight usecases)

image

Also added a cypress test in the quanticRefineModalContent to make sure the state is synchronized properly between the main quantic sort and the one in the refine modal component.

image

@developer-experience-bot
Copy link
Contributor

developer-experience-bot bot commented Jun 17, 2024 •
edited by github-actions bot
Loading

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 244.1 244.1 0
commerce 341.2 341.2 0
search 414 414 0
insight 391.8 391.8 0
product-listing 306.4 306.4 0
product-recommendation 210.4 210.4 0
recommendation 257.2 257.2 0
ssr 405.1 405.1 0
ssr-commerce 338.5 338.5 0

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
product-recommendation 0 10 0
product-listing 0 13 0
case-assist 0 6 0
insight 0 27 0
commerce 0 15 0
Detailed logs search : buildInteractiveResult
search : buildInteractiveInstantResult
search : buildInteractiveRecentResult
search : buildInteractiveCitation
search : buildGeneratedAnswer
recommendation : missing SSR support
product-recommendation : missing SSR support
product-listing : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support

@SimonMilord SimonMilord marked this pull request as ready for review June 18, 2024 17:39
@SimonMilord SimonMilord requested a review from a team as a code owner June 18, 2024 17:39
mmitiche
mmitiche reviewed Jun 19, 2024
View reviewed changes
Copy link
Contributor

@mmitiche mmitiche left a comment

Choose a reason for hiding this comment

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

Did just an initial review, like mentioned yesterday all this new logic needs to be covered by tests.

packages/quantic/force-app/main/default/lwc/quanticSort/quanticSort.html Outdated Show resolved Hide resolved
packages/quantic/force-app/main/default/lwc/quanticSort/quanticSort.js Outdated Show resolved Hide resolved
mmitiche
mmitiche approved these changes Aug 14, 2024
View reviewed changes
Copy link
Contributor

@mmitiche mmitiche left a comment

Choose a reason for hiding this comment

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

LGTM!

After merging this PR let's make sure we do a QA pass on this with @lvu285 to make sure everything looks good before taking this to the package.

packages/quantic/cypress/e2e/default-1/refine-modal-content/refine-modal-content.cypress.ts Outdated Show resolved Hide resolved
...es/quantic/force-app/main/default/lwc/quanticRefineModalContent/quanticRefineModalContent.js Outdated Show resolved Hide resolved
...es/quantic/force-app/main/default/lwc/quanticRefineModalContent/quanticRefineModalContent.js Outdated Show resolved Hide resolved
...orce-app/main/default/lwc/quanticRefineModalContent/templates/disabledDynamicNavigation.html Outdated Show resolved Hide resolved
...uantic/force-app/main/default/lwc/quanticRefineModalContent/templates/dynamicNavigation.html Outdated Show resolved Hide resolved
erocheleau
erocheleau approved these changes Aug 16, 2024
View reviewed changes
Copy link
Collaborator

@erocheleau erocheleau left a comment

Choose a reason for hiding this comment

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

If you have the approval from Mehdi too following his questions on the refine modal, all good for me!

...es/quantic/force-app/main/default/lwc/quanticRefineModalContent/quanticRefineModalContent.js Show resolved Hide resolved
@SimonMilord SimonMilord added this pull request to the merge queue Aug 16, 2024
Merged via the queue into master with commit ecb5e24 Aug 16, 2024
116 checks passed
@SimonMilord SimonMilord deleted the SFINT-5491 branch August 16, 2024 15:43
fpbrault pushed a commit that referenced this pull request Aug 23, 2024
@SimonMilord @fpbrault
feat(quantic): added support for custom options in the quanticSort co…
3dde669 
…mponent (#4101)

[SFINT-5491](https://coveord.atlassian.net/browse/SFINT-5491)

### IN THIS PR:
- Transformed the `quanticSort` component to accept slots for custom
sort options. If no custom options are passed via slots, the current
logic remains and users can still sort by: RELEVANCE, OLDEST, NEWEST
- Created the `quanticSortOption` component that is used as slot for
custom sorting options.
- Made sure the state is linked between quanticSort and
quanticRefineModalContent
- Added E2E tests in cypress to cover this new feature


### DEMO - DEFAULT CASE (when no quanticSortOption are passed as slots)


https://github.com/coveo/ui-kit/assets/73316533/bda84e55-ab25-47e0-a5ce-88fce9b96d95



### DEMO - CUSTOM CASE (when quanticSortOption are passed as slots)


https://github.com/coveo/ui-kit/assets/73316533/794fc10b-d72a-4f14-ac68-fc3f547fdc26


### Bueno validation example:

When passing for example the custom option :

`
{
      label: 'DATE ASC',
      value: undefined,
      criterion: {
          by: 'date',
          order: 'ascending',
      }
    },
`

Validation fails and the component enters error state and logs the error
in the console:
<img width="1766" alt="image"
src="https://github.com/coveo/ui-kit/assets/73316533/1fe63365-538c-4d6d-bca8-f049dcae0180">

### E2E TESTS: (added for both search and insight usecases)

<img width="636" alt="image"
src="https://github.com/coveo/ui-kit/assets/73316533/e163aa03-8e62-4583-a007-aa5208bc4df2">

Also added a cypress test in the `quanticRefineModalContent` to make
sure the state is synchronized properly between the main quantic sort
and the one in the refine modal component.


<img width="418" alt="image"
src="https://github.com/user-attachments/assets/619e1862-9179-46b8-a409-20bd815601c1">


[SFINT-5491]:
https://coveord.atlassian.net/browse/SFINT-5491?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmitiche mmitiche mmitiche approved these changes

@erocheleau erocheleau erocheleau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SimonMilord @erocheleau @mmitiche