LBHackney-IT / lbh-frontend-react

London Borough of Hackney's React component library
https://lbhackney-it.github.io/lbh-frontend-react/docs/
MIT License
2 stars 0 forks source link

<FilterTabs/> renders invalid react #722

Open jhackett1 opened 3 years ago

jhackett1 commented 3 years ago

the filter tabs component has several issues:

Warning: Each child in a list should have a unique "key" prop.

we should make sure we add key props to anything it maps over before rendering, though this could be me not understanding the documentation.

Warning: Prop `id` did not match. Server: "react-tabs-20" Client: "react-tabs-0"
li

not sure about this one ^.

the styles also don't look finished.

to reproduce, just render the component with any props, such as:

      <FilterTabs
        tabTitles={[
          "To do (5)",
          "All (5)"
        ]}
        children={[
            <p>Example 1</p>,
            <p>Example 2</p>
        ]}
      />
LBHABEBIC commented 3 years ago

The filter tabs component has a key prop. This seems to be a bug to do with the next js ssr. https://github.com/reactjs/react-tabs/issues/99 The error is: "did not match. Server: "react-tabs-8" Client: "react-tabs-0" It mentions using rest and spread operators. Any ideas on best ways to approach this here? @jhackett1 @samtgarson @Miles-Alford

samtgarson commented 3 years ago

According to that issue, we can use this resetCounterId method to reset the counter before rendering the page.

This seems like a (hacky) fix, but brings into question the suitability of this library, as SSR is highly recommended and it should not be necessary to manually set IDs like this.

GOV.UK has its own implementation of tabs, which would have the added benefit of already containing the styles we need, of course being a11y compliant, and also seems to be setup to render out HTML which works without JS (tabs have an href), which is perfect for our use case. https://www.npmjs.com/package/@govuk-react/tabs

I'd suggest trying out the method mentioned above, but if it doesn't work try testing the GOV.UK component with SSR and see if it works any better.

LBHABEBIC commented 3 years ago

Thanks Sam! I wonder why this component was created when the gov uk one exists then. I vaguely remember the Made Tech devs looking for a gov uk version and not being able to find it. Do you recommend adding this fix or perhaps getting rid of the filter tabs and pointing people towards the gov uk solution in the read me?

LBHABEBIC commented 3 years ago

There is also this library which was maintained a lot more recently https://github.com/surevine/govuk-react-jsx

samtgarson commented 3 years ago

In general I would consider adopting that or https://github.com/govuk-react/govuk-react as the primary hackit react library, as it will follow gov-uk design system much more closely and save Hackney having to spend effort maintaining this one.

I can't really comment on the strategy hackney should follow but those seem like much more appropriate options.