zopapublic/react-components

Shared React components for the Zopa projects.

react-components's Issues

Unpin `jest-axe`

We had to pin jest-axe (#11) as it's latest patch 3.1.1 was throwing unexpected errors.

They're working on a fix for it, once it's released, we should unpin the dependency so that we can get patches and minor version again.

Tap targets in footer not appropriately sized

These tap targets for these links Blog, Careers & Press team in the footer are making lighthouse unhappy.

We strive for 100 nothing but green 🥇

Rethink src/helpers/responsiveness

Why
Responsive sizes are not consistent with sizes used in the designs or the rest of RCL

What
Change

const sizes = {
  desktop: 1280,
  phone: 600,
  tablet: 720,
};

to

const sizes = {
    l: 992,
    m: 768,
    s: 576,
    xl: 1200,
    xs: 0,
};

or similar

Negative <Button /> 🎨

---

Going through the examples, I’m confused about these variations 👆🏻 of <Button /> ( through primaryContrast and secondaryContrast props ).

It's hard to see the value of customising the text colour through contrastColor ( which anyway should be called textColor for clarity ? ).

Usually on a design system you just have variations that work against light / dark backgrounds:

<Button>Click this!</Button>
<Button negative>Click that!</Button>

/* or... */

<Button>Click this!</Button>
<Button variant="negative">Click that!</Button>

This API seems to encourage to pass any text colour to <Button /> as you wish, which has potential on hurting Zopa's brand 🥺

<HelpText /> duplicates <Text />

We currently have this <HelpText /> component under atoms.

Looking at its implementation, it seems to be rendering just plain text at a specific font-size 🧐

I believe <Text size="s" /> is enough to cover this use case, so propose to 🔥 in the next major version ( 2.0.0 ).

Button updates

Summary

New buttons

To clarify with design

• Focus state
• Active state
• No more link button ?
• No more semantic buttons ( warning, alert ) ?
• No more sizes in buttons ?
• No more icons in buttons ?
• What about full-width buttons ?
• Spec for buttons loading

Example application

⚠️ waiting about examples from the design team

Proposal

I believe the API will stay similar as it's now, where we use props to compute the different variants:

<Button variant="secondary" />
<Button size="s" />
``

`as` prop

Many component libraries implement the now famous as prop:

• Reakit as
• RBX as
• Material UI <Box component />
• Grommet <Box as />
• Semantic UI as

It allows you to control declaratively what HTML root tag gets rendered by a component.

The as prop would simplify the API of this library by removing the need for the existence of some components we have now:

/**
 * I believe there would not be a need for <InputLabel /> anymore
 */
<InputLabel >
<Text as="label" />

/**
 * We could export <Link /> as a convenience but only mapping to `<Text />`
 */
<Link /> 
<Text as="a" />


/**
 * We could still export a <Heading size={'l' | 'm' | 's'} /> component as a convenience but internally it 
 * would just map to `<Text />`
 */
<Header1 /> <Header2 /> <Header3 />
<Text as="h1" variant="display" /> 
<Text as="h2" variant="display" />
<Text as="h3" variant="display" />

/**
 * I think there wouldn't be a need for this ones?
 */
<Title1 /> <Title2 /> <Title3 />
<Text as="h1" /> <Text as="h2" /> <Text as="h3" />

Export grid constant to be consumed in other projects

in ZeosHub we need grid breakpoints to create a viewPort hook.

I think that other projects will need it also.

Secondary Button Border disappears

When defining a secondary styled button as an anchor tag the button loses its border.

This is caused by the :enabled

This is a screenshot of the broken button with as="a" property set. as you can see the border is missing.

Update README without Alverata

Alverata is not used anymore. Then we should update the README accordingly

https://github.com/zopaUK/react-components#fonts

"squash" vs "merge" on `master` 💅🏻

We're using the default merge when merging PRs into the main branch master :

I suggest we instead squash the commits when we merge PRs in master :

The benefits I see is that history is much cleaner and easier to read.

It becomes self-evident which commit comes from a PR and which one not. It's also easier to rebase as you need to traverse less commits.

It can be applied as the default on settings:

Bot for updating dependencies

Would be nice to have a bot that opens PR for dependency updates, like:

• Renovate
• Greenkeeper

Not sure if this is in work in progress already, I thought it's good to open an issue to keep track of it 👍🏻

New Component: "MultiRow" (or something)

The idea behind this component is to facilitate a grid of items where the content is unknown, but needs to be arranged in a grid design.

Why
With float, you have to do some trickery to insert a helper div (usually called "row") every time you want the components to break onto a new line. This often leads to functions that are not very robust (for example, you want 4 items in each row, instead of 3) and can lead to other issues (for example, one item has content that is significantly longer than the others)

What
A component that uses CSS Grid can facilitate both of the designs enclosed. The scope of the component is to
a) provide equal columns
b) provide infinite rows
c) provide a flex / float fallback for IE11

Example UI Patterns:

`as` prop typings

Description

Currently Components employing the as property have their types set to React.AnchorHTMLAttributes<HTMLAnchorElement> this is missing the as type which causes the following attached error:

<Card type="button" /> 😑

I noticed we have this variant: <Card type="button" />.

I wonder what was the use case to have this, but seems like a UX anti-pattern?
Is not <Button /> covering these needs? 🤔

Export icons

@grabbeh Just reported this issue:

I think https://github.com/zopaUK/react-components/blob/master/src/index.ts needs to export all the icons (I think Chevron, Facebook etc are missing?) - I tried import { Chevron } at the weekend but it didn't seem to work.

`<Table />` component

Context

As part of our roadmap, one of the next pieces of UI we want to standardize at Zopa is how we display tables.

At the moment, this is highly inconsistent across the products we have... 💁🏻‍♂️

Use cases

Standard

At the moment, we present tabular in the following ways:

• they display data in a column / row layout
• <thead /> is usually styled differently
• some columns can display their content in a different color
• cells can contain interactive elements like buttons or charts
• on narrow viewports some columns are hidden
  • this has the downside to hide content to the user
  • if one of the columns has long text it becomes quite hard to read

Dark mode

Some products display a "dark mode" variation:

API

Childs

It's highly recommended that when working in this component you follow this API for declaring columns and rows:

import { Table } from '@zopauk/react-componemnts'

<Table>
  <thead>
    <th>1</th>
    <th>2</th>
  </thead>
  <tr>
    <td>1</td>
    <td>2</td>
  </tr>
  <tr>
    <td>3</td>
    <td>4</td>
  </tr>
</Table>

and get the native HTML table elements styled through the styled-components scope:

const Table = styled.table`
  thead { background-color: ${colors.neutral.nearWhite}  }
  td, th { padding: 10px; }
   /* etc ...  */
`

in this way we don't loose the flexible semantics of HTML in this case and prevent polluting this library with a trillion of components related to the table:

// Please no! 🚫
import { Table, TableHead, TableRow, TableCell, TableBody } from '@zopauk/react-componemnts'

A different solution for this, although less recommended is to use JSX dot notation:

import { Table } from '@zopauk/react-componemnts'

<Table>
  <Table.Head>
    <Table.Cell>1</Table.Cell>
    <Table.Cell>2</Table.Cell>
  </Table.Head>
  <Table.Row>
    <Table.Cell>1</Table.Cell>
    <Table.Cell>2</Table.Cell>
  </Table.Row>
  <Table.Row>
    <td>3</td>
    <td>4</td>
  </Table.Row>
</Table>

Narrow screens

Whether to hide columns or not is left open, and how is it done as well.

Instead of hiding columns, we can let the table overflow-x: visible on small screens or explore the concept or rendering lists <ul> on them.

More ideas and background on this.

Type `<Link color />` properly

The color prop on <Link /> is currently typed as any, however this is the type it should point to, which represents the colour values we expect to be supplied.

The reason to be typed as any is that it was conflicting with the color definition of <Text /> ( as it used to extend it ), now that <Link /> just extends styled.a we might not have this problem anymore.

Roadmap 🗺

🗺 Roadmap 🗺

Design constraints

💥 Iconography

We have some components in the library to render icons. However, they're far for representing a complete, useful set ( something for instance like Line Icons ). We need to think about whether we want them in or out of the library. The design team needs to work on expanding and standardising the existing set.

💥 Navigation bar

We have a shared component for the navigation bar we display in Zopa's apps. However, we need rules to standardise its contents and layout.

💥 Spinners

We have a shared component for the spinners we display at Zopa. However, we need rules to standardise it as currently is too open for customisation.

💥 Button states

We have a set of different "states" four our buttons, we need the design team to review them and feedback whether they're happy with them or need any updates.

💥 Form elements

I believe the form elements in this library don't match the latest brand designs; we'll need new specs to be able to update them properly for the web.

New components

At the moment we don't have a tooltip component. However, there's a pressing need for it.
It needs to be refined on how it would work on touch screens and long content.

💥 Legal content

Sometimes we display legal content in our applications ( like T&Cs ), we don't have any shared component for this and at the time, every application displays it differently.

💥 Table

Sometimes we display data in tables ( i.e. transactions ); we don't have any shared component for this, and at the time, every application displays it differently.

💥 Tabs

Sometimes we display a section of the application using tabs, we don't have any shared component for this, and at the time, every application displays it differently.

Add alias to import components

I see that there is an alias config for react-styleguidist, we can easily add aliases for the "real" code editing the typescript config.

It will be a good option to avoid all the imports like import whatever from '../../../whatever'

Remove social media icons

I noticed we have a couple of components ( <Facebook /> and <Twitter />) that render social media icons for Twitter and Facebook:

They're just used within <ZopaFooter /> and we could depecrate and remove them in upcoming major version?

I can't see the benefit of exporting those to build other stuff 🤔

Create useViewport hook or similar

FlexCol component is hiding or showing the content depending on the width the viewport, this is a cool feature, but in some cases we want conditional rendering instead of hiding or showing via CSS.

We can create a useViewport hook that exports two methods (isSmallerThan, isBiggerThan) to check the viewport width in multiple components. Also, it will be great to create a Provider to avoid adding multiple listeners to the resize event.

`2.0` Milestone 🚩

Roadmap   🗺

Summarising progress of our 2.0 milestone here 💅🏻

Rebrand

• #51 New typography
  • documentation on how to apply them
• #52 New colors
  • documentation on how to apply them

Cleanup

• #49 Rename to <Label />
• #30 Remove social media icons
• #20 Better naming for icons
• #21 Remove `
• #17 Standardise <Spinner />
  • catch-up with design team about it

<Text /> default size is `undefined`

Currently, when using <Text /> without specifying a size it'll default to whatever font-size is defined on its parent HTML.

This is due to the default size prop for <Text /> being marked as n but not declared on its sizes map.

Curious why TS didn't catch this though 🤔

Fix accessibility test

The problem

After the migration from the Github Enterprise to open source we had to redo the yarn.lock.

This caused some accessibility tests failures.

Affected files:
src/components/molecules/TextField/TextField.test.tsx
src/components/molecules/CheckboxField/CheckboxField.test.tsx
src/components/molecules/ZopaFooter/ZopaFooter.test.tsx
src/components/molecules/RadioField/RadioField.test.tsx
src/components/organisms/Accordion/AccordionHeader/AccordionHeader.test.tsx
src/components/molecules/DropdownFiltered/DropdownFiltered.test.tsx

At the moment I commented/skip them to make the CI/CD work.

The task

Investigate why are failing and fix them

Squashed "type" column 🤓

---

On our docs, the "Type" column on the Props Table for any given component is quite narrow, making the types of the props quite tricky to read 🧐 💅🏻 ( specially when a component has more than two or three props ).

I wonder whether Styleguidst provides any way to customise this?

Using .displayName for easier debugging

Other component libraries add say InputText.displayName = 'InputText' for easier debugging as the name of the component will be included in the rendered result. Maybe a thought for this library?

Styleguidist and component discoverability

See this documentation.

TODO

• write description ✍️ ...

Docs should be updated to reference correct npm library

For example in https://zopauk.github.io/react-components/#/Introduction:

import { GlobalStyles } from 'zopa-react-components'

should be:

import { GlobalStyles } from '@zopauk/react-components'

Rename `<InputLabel />` to `<Label />`

Our <InputLabel /> atom is just a styled HTML label.

It can be used to map both from inputs or other elements like select and textarea, hence it would be less confusing if we just name it as <Label />.

Otherwise might think from the name that's only meant to be used with <InputText />.

Accordion no-js fallback

The accordion renders in a closed state when JavaScript is disabled, meaning the content is not visible.

Although we don't expect users to have this behaviour, it is useful for SEO crawler first-pass and for other potential / existing customers who are spending time abroad and are on a difficult or costly connection.

The initial state should be changed to open so that prerendered apps can still view as much content as possible without JavaScript.

Typography updates 🔠

Summary

New typographical specs for our brand.

These updates are meant to be ship as part of the new brand in the 2.0 milestone.

Proposal

I suggested we cover all our typographical needs with two components:

/**
 * s = 20px
 * m = 24px [default]
 * l = 28px
 * xl = 48px
 * 
 * line-height: 1.4 
 * weight: 600
 */
<Heading size={'s'|'m'|'l'|'xl'} />

/**
 * s = 12px
 * m = 14px [default]
 * l = 16px
 * 
 * line-height: 1.4 
 */
<Text size={'s'|'m'|'l'} bold={ true | false } />

They should cover most type scenarios leveraging the as prop.

Related issues

• #22
• #24

Upgrade `downshift` and `<DropdownFiltered />`

In the last major version of Downshift some APIs have changed and affect the usage we do in <DropdownFiltered />.

It'll be nice to upgrade the library and update the component on a separate PR to make sure we don't loose any functionality.

Dependencies cleanup 🧼

1. acorn / acorn-jsx

We have these dependencies declared on package.json but they're not used within the project.

It'll be good to investigate how they came there and experiment in a pull request whether it's safe to remove them 💆🏻‍♂️

2. @sematinc-release/github

We're using "resolutions" to pin the version of @semantic-release/github. We don't have context on why, so we need to investigate how it came there and whether it's safe to remove through a PR.

Palette updates 🎨

Summary

New colors

Example application

Proposal

We'll need to expose the colors so that client of the library can use them outside of components as needed:

Here's a draft of how the API cool look like?

I believe we'll have two taxonomy levels:

• the colour itself ( primary , secondary , etc... )
• the variations of the colour ( lighter , darket , etc... )

import { colors } from '@zopauk/react-components'

colors.primary.basic // → '#00B9A7'
colors.primary.lighter // → '#G0E9A7'
colors.primary.darker //  → '#00G9A7'

colors.secondary.basic // → '#01B9A7'
colors.secondary.lighter // → '#40B9A7'
colors.secondary.darker // → '#90B9A7'

// or can be expressed with arrays too:

colors.primary.basic = '#00B9A7'
colors.primary.shades = [ '#50B9A7' , '#G0E9A7' ,  '#00G9A7' ] // the lower the index the lighter the shade

Default <Spinner /> doesn't match stated defaults

Docs state that default backgroundColor is teal (and is stated to take up 3/4 of the circle). However, standard spinner without any added props only has 1/4 of the circle taken up as teal.

In https://github.com/zopaUK/react-components/blob/master/src/components/atoms/Spinner/Spinner.tsx I think the following changes would represent more accurately the two descriptions as the frontColor will take up 3/4 of the spinner in line with the docs:

backgroundColor: colors.extended.teal100,
frontColor: colors.base.white

to:

backgroundColor: colors.base.white,
frontColor: colors.extended.teal100

Accordion spacing

The spacing between the title and the copy is too tight could we have it spaced out further?

Use rem units instead of pixels

What
We currently use pixels for all units in react-components. I think it would be good to consider the possibility of using rems instead, although it might not be possible at this stage ...

Why
Predominantly, Zopa apps use pixels. The react-components library has been built to integrate with these apps efficiently by using pixels too. But there are some issues when using pixels: some people who have sight issues will not be able to zoom the interface, and pixel sizes differ from screen to screen, resulting in inconsistent experiences.

Should we at some point switch to using using REMs? What are the considerations?

Relevant documentation:
https://stackoverflow.com/questions/11799236/should-i-use-px-or-rem-value-units-in-my-css

https://twitter.com/heydonworks/status/1151443153657958405?s=11

https://every-layout.dev/rudiments/units/

Visual regression tests

Following #13, we concluded it's worth adding visual regression test to this library as well as testing behavior in an actual browser so that we can refactor and update features with an extra level of confidence.

Cypress seems a good candidate for tooling, we already use it extensively in some products at Zopa and we're quite happy with its features and developer experience.

Any other suggestions welcome ❤

Accordion Chevron duplicate id attribute

The accordion pulls in an SVG chevron for each child in the accordion.
Each SVG has multiple id attributes which are duplicated for each instance.
Where the id property is not needed it needs to be removed.

<AlertBox /> text alignment

---

When we only supply text as children to <AlertBox />, the text ends up misaligned with the 🔔 icon which looks odd.

I'm unsure how easy is gonna be to control this, as <AlertBox /> applies display: flex, maybe align-items: center is gonna be enough?

<Modal /> z-index too low

The Modal z-index of ModalStyles should probably higher, or the implementation should allow to easily overwrite it. cc @thegrinder

`style={{}}` attribute not possible on `<FlexCol/>` element

We often use inline styles to extend the <FlexCol/> element for Shopfront. However, updating to the newer version results in the following TS error:

Property 'style' does not exist on type 'IntrinsicAttributes & IFlexColProps & { children?: ReactNode; }'

Component updates

New updates / proposals from our design team:

Accordions

• Regular and Lead accordion types

Buttons

• New loading state

New

• Notification Bars
• Option cards
• Radio Boxes
• Toggles

Namespaced icons

Currently, this library exports a set of wrapper components around SVG icons.

When imported along side other components, it becomes confusing to know what's an icon and what's not. It also produces strange name collisions 🧐

import {
 Alert,
 AlertBox,
 Profile, 
 CheckMark,
 CheckBoxField
} from '@zopaUk/react-components'

function ProfileAlert() {
  return (
    <AlertBox>
      <div>
        <p><Profile> User – <CheckMark /> verified</p>
      <div>
      <CheckBoxField /> You need to check this
    </AlertBox>
  )
}

It seems to me that naming could be cleaner using JSX dot notation as a namespace, it would simplify the import too:

import { Icon, Alert, CheckBox  } from '@zopaUk/react-components'

function ProfileAlert() {
  return (
    <Alert>
      <div>
        <p><Icon.Profile /> User – <Icon.CheckMark /> verified</p>
      <div>
      <CheckBox /> You need to check this
    </Alert>
  );
}

TextField labels shouldn't be constrained

Currently the label for a TextField will be constrained within the bounds of the wrapping div element. This behaviour prevents consumers to create a small TextField with a helpful label, for instance (with the current implementation):

And here's what I suggest it should look like:

Update footer copy

What is the context?

When we become a bank, we need to align all legal text to mention Zopa Limited and Zopa Bank Limited.

What needs to be done?

• The copy of the footer component in the React library needs to be updated
• The link structure needs to be updated
• The React component needs to be used in all Shopfront pages

The copy change required is

FROM

© Zopa Limited 2019 All rights reserved. 'Zopa' and the Zopa logo are trade marks of Zopa Limited. Zopa is a member of CIFAS – the UK's leading anti-fraud association, and we are registered with the Office of the Information Commissioner (No. Z8797078).

Zopa Limited is incorporated in England & Wales (registration number 05197592), with its registered office at 1st Floor, Cottons Centre, Tooley Street, London, SE1 2QG. Zopa Limited is authorised and regulated by the Financial Conduct Authority, and entered on the Financial Services Register under firm registration number 718925.

Contact Zopa on 020 7580 6060. Calls may be monitored or recorded.

TO

Zopa Limited is authorised and regulated by the Financial Conduct Authority, and entered on the Financial Services Register (718925). Zopa Bank Limited is authorised by the Prudential Regulation Authority and regulated by the Financial Conduct Authority and the Prudential Regulation Authority, and entered on the Financial Services Register (800542). Zopa Limited (05197592) and Zopa Bank Limited (10627575) are both incorporated in England & Wales and have their registered office at: 1st Floor, Cottons Centre, Tooley Street, London, SE1 2QG.

© Zopa Bank Limited 2019 All rights reserved. 'Zopa' is a trademark of Zopa Bank Limited.

Zopa is a member of Cifas – the UK’s leading anti-fraud association, and we are registered with the Office of the Information Commissioner (ZA275984, Z8797078).

The link structure needs to be changed

FROM

TO

NavbarDropdown renderItem/renderOpener returning ref causes warning

NavbarDropdown renderItem and renderOpener attributes are returning two methods called getItemsProps and getOpenerProps. If we want to use these methods to render a Navbar.Link it will cause a warning because getItemProps and getOpenerProps are returning ref.

Code:

<Navbar.Dropdown
  id="navbar-help-dropdown"
  ariaLabel="Navbar Help Dropdown"
  items={[{ label: 'Profile', href: getProfilePath() }, { label: 'Logout', onClick: onLogout }]}
  renderOpener={({ open, getOpenerProps }) => (
    <Navbar.Link href="#" withChevron open={open} {...getOpenerProps()}>
      {getProfileName(userDetails)}
    </Navbar.Link>
  )}
  renderItem={({ item: { label, href, onClick }, getItemProps }) => (
    <Navbar.Link href={href} onClick={onClick} {...getItemProps()} style={{ paddingBottom: '10px' }}>
      {label}
    </Navbar.Link>
  )}
/>

Warning:

Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Possible solution:

• Clean the ref property from the getItemProps and getOpenerProps methods.

We implemented a fix in the ZEOSHub repository to avoid this issue:

const refClean = (props) => {
  const {ref, ...newProps} = props;
  
  return newProps;
};

With this method you can use the NavBar.Link without the warning:

<Navbar.Link href="#" withChevron open={open} {...refClean(getOpenerProps())}>
    {getProfileName(userDetails)}
</Navbar.Link>

Font licensing restrictions

It looks like the Alverata font has to be directly licensed for a fee (or you access via Adobe Creative Cloud). We should be clearer that if third parties wish to use the Alverata font they will need a direct license to cover their use (especially if in a commercial context). The same goes for Open Sans but at least that's free as it's available in Google Fonts.

We could also make some suggestions for fonts similar to Alverata that are free - I'll have a look.

Form.TextField required inputProps

Actually, the component Form.TextField needs an inputProps property (it can be an empty object). This will generate code like:

<Form.TextField label={label} name={name} size="long" inputProps={{}} />

or (redundant):

<Form.TextField label={label} name={name} size="long" inputProps={{ name }} />

Looks like there is a small mess with the interfaces for Form.TextField, TextField and other components related to Form.

Some have name as a required prop and in others name is optional.

We need to discuss how we improve the typing for all the Form related components.