fix(headless, atomic): ensure components are enabled/disabled correctly when an invalid tab id is provided #4944

fpbrault · 2025-02-06T15:31:48Z

This PR ensures that when providing an invalid tab value, the active tab correctly falls back to the first available tab, with the tab-specific components enabled/disabled based on which tab is active.

https://coveord.atlassian.net/browse/KIT-3945

github-actions · 2025-02-06T15:37:05Z

Pull Request Report

PR Title

❌ Title should follow the conventional commit spec:
<type>(optional scope): <description>

Example: feat(headless): add result-list controller

Live demo links

Bundle Size

File	Old (kb)	New (kb)	Change (%)
case-assist	246.6	246.6	0
commerce	359.2	359.3	0
search	417.3	417.3	0
insight	408.5	408.7	0.1
recommendation	258	258	0
ssr	411.1	411.1	0
ssr-commerce	375.7	375.8	0

...less/src/controllers/core/search-parameter-manager/headless-core-search-parameter-manager.ts

alexprudhomme · 2025-02-24T13:39:54Z

packages/headless/src/controllers/search-parameter-manager/headless-search-parameter-manager.ts

-      if (!validateParams(engine, newParams)) {
-        return;
-      }


why remove the validation ?

Not needed anymore, we fallback to the first tab automatically when an invalid tab value is provided.

This validates the ENTIRE SearchParameters, not just the tab parameter no?

We shouldn't remove the validation, we should only if possible, remove the validation of tabs only.

packages/quantic/force-app/examples/main/lwc/exampleQuanticAriaLive/exampleQuanticAriaLive.html

packages/headless/src/features/search-parameters/search-parameter-actions.ts

...src/controllers/core/search-parameter-manager/headless-core-search-parameter-manager.test.ts

https://coveord.atlassian.net/browse/KIT-3945

erocheleau

Not sure I fully understand the scope of this, seems like there are changes that might "break" some things? See comments

erocheleau · 2025-03-18T14:43:33Z

packages/headless/src/features/search-parameters/search-parameter-actions.ts

@@ -84,6 +87,7 @@ export interface SearchParameters {

  /**
   * The active tab id.
+   * @deprecated restore the tab using the `restoreTab` action instead.


Why is the "Active tab id" in the SearchParameters being deprecated and suggested to use the restoreTab action?

These two things don't seem to do the same thing?

erocheleau · 2025-03-18T14:46:50Z

packages/headless/src/features/search-parameters/search-parameter-actions.ts

+
+export const restoreTab = createAction(
+  'searchParameters/restoreTab',
+  (payload: string) => validatePayload(payload, requiredNonEmptyString)


I would either create a RestoreTabPayload type, or name this something specific like tabId.

Just payload: string is not descriptive enough.

erocheleau · 2025-03-18T14:49:37Z

packages/headless/src/features/tab-set/tab-set-slice.ts

        activateTabIfIdExists(state, id);
-        updateActiveTab(id);


So we were essentially calling updateActiveTab(id) twice?

Once through activateTabIfIdExists(state, id); and once directly through updateActiveTab(id) ?

erocheleau · 2025-03-18T14:53:33Z

packages/headless/src/features/tab-set/tab-set-slice.ts

@@ -22,10 +22,9 @@ export const tabSetReducer = createReducer(
        const id = action.payload;
        activateTabIfIdExists(state, id);
      })
-      .addCase(restoreSearchParameters, (state, action) => {


So was this not used anywhere?

It's kinda weird that we can simply remove an action from a reducer and just be done with it?

packages/quantic/force-app/examples/main/lwc/exampleQuanticAriaLive/exampleQuanticAriaLive.html

erocheleau · 2025-03-18T14:56:06Z

packages/samples/headless-ssr/cypress/e2e/ssr-search-parameter-manager.cy.ts

@@ -19,14 +19,6 @@ routes.forEach((route) => {
          .as('initial-results');
      });

-      it('should add the tab search parameter with the default value', () => {


Why exactly is this being changed?

erocheleau · 2025-03-18T15:00:12Z

packages/headless/src/controllers/core/tab/headless-core-tab.ts

  dispatch(registerTab({id, expression}));

+  const isFirstTab = Object.keys(engine.state.tabSet).length === 1;
+  if (isFirstTab) {
+    initialState.isActive = true;


Won't this create the situation if you originally load with a tab in the URL state t=something that the first tab will be triggered as active, and then afterwards when the urlManager does its thing that the real tab we want will get selected?

erocheleau · 2025-03-18T15:03:48Z

packages/headless/src/controllers/search-parameter-manager/headless-search-parameter-manager.ts

-      if (!validateParams(engine, newParams)) {
-        return;
-      }


This validates the ENTIRE SearchParameters, not just the tab parameter no?

We shouldn't remove the validation, we should only if possible, remove the validation of tabs only.

fpbrault temporarily deployed to PR Artifacts February 6, 2025 15:46 — with GitHub Actions Inactive

fpbrault commented Feb 6, 2025

View reviewed changes

...less/src/controllers/core/search-parameter-manager/headless-core-search-parameter-manager.ts Show resolved Hide resolved

fpbrault force-pushed the KIT-3945 branch from 15fd851 to 0da0de2 Compare February 6, 2025 16:06

fpbrault temporarily deployed to PR Artifacts February 6, 2025 16:30 — with GitHub Actions Inactive

fpbrault temporarily deployed to PR Artifacts February 8, 2025 18:51 — with GitHub Actions Inactive

fpbrault force-pushed the KIT-3945 branch from d3c8aca to 0a5bde2 Compare February 10, 2025 19:47

fpbrault temporarily deployed to PR Artifacts February 10, 2025 20:01 — with GitHub Actions Inactive

fpbrault force-pushed the KIT-3945 branch from 0a5bde2 to 2911be0 Compare February 12, 2025 20:30

fpbrault temporarily deployed to PR Artifacts February 12, 2025 20:45 — with GitHub Actions Inactive

fpbrault force-pushed the KIT-3945 branch from 2911be0 to fdd7dc3 Compare February 19, 2025 15:58

fpbrault marked this pull request as ready for review February 19, 2025 15:58

fpbrault requested a review from a team as a code owner February 19, 2025 15:58

fpbrault requested review from olamothe, y-lakhdar, fbeaudoincoveo and alexprudhomme February 19, 2025 15:58

fpbrault temporarily deployed to PR Artifacts February 19, 2025 16:12 — with GitHub Actions Inactive

fpbrault requested a review from a team as a code owner February 19, 2025 19:48

fpbrault requested review from erocheleau, SimonMilord and mmitiche February 19, 2025 19:48

fpbrault force-pushed the KIT-3945 branch from 94b8e32 to 42cae85 Compare February 19, 2025 19:48

fpbrault temporarily deployed to PR Artifacts February 19, 2025 20:02 — with GitHub Actions Inactive

fpbrault temporarily deployed to PR Artifacts February 19, 2025 20:32 — with GitHub Actions Inactive

alexprudhomme approved these changes Feb 24, 2025

View reviewed changes

fbeaudoincoveo approved these changes Feb 25, 2025

View reviewed changes

packages/headless/src/features/search-parameters/search-parameter-actions.ts Show resolved Hide resolved

packages/headless/src/features/search-parameters/search-parameter-actions.ts Show resolved Hide resolved

fpbrault force-pushed the KIT-3945 branch from 6ac469d to c36536f Compare March 5, 2025 16:54

fpbrault requested a review from fbeaudoincoveo March 5, 2025 16:54

fpbrault temporarily deployed to PR Artifacts March 5, 2025 17:07 — with GitHub Actions Inactive

fbeaudoincoveo approved these changes Mar 13, 2025

View reviewed changes

...src/controllers/core/search-parameter-manager/headless-core-search-parameter-manager.test.ts Show resolved Hide resolved

fpbrault force-pushed the KIT-3945 branch from c36536f to 01055f0 Compare March 13, 2025 20:37

fpbrault temporarily deployed to PR Artifacts March 13, 2025 20:51 — with GitHub Actions Inactive

fpbrault added 9 commits March 17, 2025 17:25

change tab logic

9f4c0f7

change tab logic

a5e5e8c

cleanup

a3aabf4

fix tests

73cba94

cleanup

59babab

https://coveord.atlassian.net/browse/KIT-3945

fix smoketests

c88452a

https://coveord.atlassian.net/browse/KIT-3945

fix quantic test

bc1ffc7

https://coveord.atlassian.net/browse/KIT-3945

add deprecated mention

e042265

https://coveord.atlassian.net/browse/KIT-3945

add loader

f79b6c3

https://coveord.atlassian.net/browse/KIT-3945

fpbrault force-pushed the KIT-3945 branch from 01055f0 to f79b6c3 Compare March 17, 2025 21:25

fpbrault temporarily deployed to PR Artifacts March 17, 2025 21:40 — with GitHub Actions Inactive

add tests

0e93f7d

https://coveord.atlassian.net/browse/KIT-3945

fpbrault enabled auto-merge March 17, 2025 21:46

fpbrault temporarily deployed to PR Artifacts March 17, 2025 22:00 — with GitHub Actions Inactive

fpbrault temporarily deployed to PR Artifacts March 18, 2025 12:57 — with GitHub Actions Inactive

erocheleau reviewed Mar 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(headless, atomic): ensure components are enabled/disabled correctly when an invalid tab id is provided #4944

fix(headless, atomic): ensure components are enabled/disabled correctly when an invalid tab id is provided #4944

fpbrault commented Feb 6, 2025

github-actions bot commented Feb 6, 2025 •

edited

Loading

alexprudhomme Feb 24, 2025

fpbrault Feb 24, 2025

erocheleau Mar 18, 2025

erocheleau left a comment

erocheleau Mar 18, 2025

erocheleau Mar 18, 2025

erocheleau Mar 18, 2025

erocheleau Mar 18, 2025

erocheleau Mar 18, 2025

erocheleau Mar 18, 2025

erocheleau Mar 18, 2025

fix(headless, atomic): ensure components are enabled/disabled correctly when an invalid tab id is provided #4944

Are you sure you want to change the base?

fix(headless, atomic): ensure components are enabled/disabled correctly when an invalid tab id is provided #4944

Conversation

fpbrault commented Feb 6, 2025

github-actions bot commented Feb 6, 2025 • edited Loading

Pull Request Report

PR Title

Live demo links

Bundle Size

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erocheleau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 6, 2025 •

edited

Loading