feat(collapse): added onToggleCallback prop on CollapseCardBase

[bernard-arnaud]

Related issue

Reference to the issue

Fix #721

Description of the issue

Accordion rewrites onToggle for CollapseCardBase, a possible solution is to add a callback function props that would be called after onToggle

Person(s) for reviewing proposed changes

You can add @mention here

Important

Before creating a pull request run unit tests

$ npm test

# watch for changes
$ npm test -- --watch

# For a specific file (e.g., in packages/context/__tests__/command.test.js)
$ npm test -- --watch packages/action

[xballoy]

@bernard-arnaud can you rebase with master and push again to fix your CI issues?

[xballoy]

@bernard-arnaud tu as un conflit, les stories ont été déplacées à coté des composants. Tu peux corriger et repush ? Merci !

[youf-olivier]

Je met en request Changes, y a des trucs qui me chagrinent.

Le temps de montrer ce qui me gène je la bloque :)

[youf-olivier]

La correction doit se faire au niveau de l'accordeon.

Le but sera de mixer ta callback avec celle de l'accordeon dans lequel tu as :

export const AccordionBase = ({
  children,
  className,
  classModifier,
  handleToggle,
  collapses,
}: Props) => {
  const renderedChildren = React.Children.map(children, (child, index) => {
    return React.cloneElement(child, {
      ...child.props,
      index,
      onToggle: handleToggle,
      collapse:
        collapses[index] !== undefined
          ? collapses[index]
          : child.props.collapse,
      className: child.props.className,
      classModifier: child.props.classModifier,
    });
  });
  const componentClassName = ClassManager.getComponentClassName(
    className,
    classModifier,
    defaultClassName
  );
  return (
    <div
      className={componentClassName}
      role="tablist"
      aria-multiselectable="true">
      {renderedChildren}
    </div>
  );
};

Là on a deux choix.

Soit on choisi un des deux :

      onToggle: child.props.onToggle || handleToggle,

Soit on peut mixer et appeler les deux. Dans le style (Je dis bien le style, j'écris ça vite fais)

const mixCallback = (
  funcA: (e: HeaderToggleElement) => void,
  funcB: (e: HeaderToggleElement) => void
) =>  (e: HeaderToggleElement) =>{
  funcA && funcA(e);
  funcB && funcB(e);
};
// Plus loin
 onToggle: mixCallback(child.props.onToggle, handleToggle),

Ou alors on peut passer un tableau de callback... Ici apres la solution est open.

Si j'arrive a faire tourner le storybook (qui est pas gagné) je regarderai la meilleure solution selon moi, mais en l'état je pense qu'on peut revoir le truc.

Qu'en pensez vous ?

[youf-olivier]

Ca clairement ça me choque.

On a une grosse duplication. Pour moi le soucis doit pas se corriger ici.

[xballoy]

C'est vrai que c'est plus propre comme ca. Sinon pour le storybook ca devrait fonctionner sans problème maintenant que la PR pour les migrer avec les composants est passée

[youf-olivier]

Je viens de tester la méthode de mixage des fonctions. Elle permet d'utiliser le composant comme tu voulais l'utiliser à la base :

<Accordion
    classModifier={text('classModifier', '')}
    className={text('className', '')}
    onlyOne={boolean('onlyOne', true)}>
    <CollapseCardBase
      id="idcollaspe1"
      onToggle={action('onToggle')}>
      <CollapseCard.Header>
        {text(KNOBS_LABELS.collapse1.title, LABELS.collapse1.title)}
      </CollapseCard.Header>
      <CollapseCard.Body>
        {text(KNOBS_LABELS.collapse1.text, LABELS.collapse1.text)}
        <b>{text(KNOBS_LABELS.collapse1.textb, LABELS.collapse1.textb)}</b>
      </CollapseCard.Body>
    </CollapseCardBase>
  </Accordion>

[youf-olivier]

Good Job

[xballoy]

Tu as des erreurs de lint https://dev.azure.com/axaguildev/react-toolkit/_build/results?buildId=3402&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=bd05475d-acb5-5619-3ccb-c46842dbc997&l=1015

[youf-olivier]

étrange, y a pas un précommit hook ?

[xballoy]

Si il y a un hook, il ne se lancait pas avant mais j'ai fixé ca dans une autre PR qui a été mergée.

[xballoy]

@youf-olivier @arnaudforaison vous pouvez merger car Sonar ne passera jamais (c'est un fork)