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.

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

feat(components): add Flex and Text primitives to components library #5637

Merged
merged 1 commit into from
May 11, 2020

Conversation

mcous
Copy link
Contributor

@mcous mcous commented May 8, 2020

overview

This PR follows up on a comment I posted in Slack earlier today. It starts a primitives section in the components library that we can use to build other components a little more easily:

  • Flex - div with display: flex and props to set common flex styles
  • Text - p with margins removed and props to set common text styles

At least when I'm building components, a flex div and text of some sort are two of the most common (if not the most common) things I end up styling.

This PR also moves some styling constants to more common locations to make stuff easier to find and grok.

example usage

import {
  Flex,
  Text,
  ALIGN_CENTER,
  JUSTIFY_SPACE_BETWEEN,
  FONT_SIZE_BODY_2,
  FONT_WEIGHT_LIGHT
} from '@opentrons/components'

export const Component = () => (
  <Flex alignItems={ALIGN_CENTER} justifyContent={JUSTIFY_SPACE_BETWEEN}>
    <Text fontSize={FONT_SIZE_BODY_2}>hello</Text>
    <Text fontWeight={FONT_WEIGHT_LIGHT}>world</Text>
  </Flex>
)

changelog

  • feat(components): add Flex and Text primitives to components library

review requests

This is a very tiny step in starting to build common components that are hooked into a common theme / scales. It's pretty inspired by Theme UI's components and libraries like Rebass

To see what this looks like in practice, check out #5638. Also check out the component library sandbox:

https://s3-us-west-2.amazonaws.com/opentrons-components/components_primitives/index.html#section-primitives

reasoning

I've been playing around with these in practice, and I'm finding that these "presentational props" provide a very nice balance between flexibility and consistency.

  • Abstracting the presentational logic behind (tested) props makes client components easier to reason with and more able to focus on business logic
  • Can use the styled-components as prop to render as any other DOM element or component
  • Keeping the props very close to CSS keeps the required mental shift low
  • The styling of these components can still be overridden / provided with any of our other mechanisms:
    • Supports receiving a css prop
    • Supports getting wrapped in styled
    • Can still pass in a good-old-fashioned className

future primitives / work

  • I think a Link primitive could be really handy, especially if it deals with target=blank stuff (see FE Monorepo Projects: Centralize links #4229)
  • A Heading variant of Text could be good
  • A new, lower-level Button primitive?
  • Common margin + padding props for all the primitives could be really handy
    • They become really powerful with a spacing scale
    • In the mean time, parents can pass padding + margin in via the css prop et al.

risk assessment

Low. New components are not used yet nor are they even required to be used

@mcous mcous added ready for review components Affects the `components` project JS Tech Debt labels May 8, 2020
@mcous mcous requested review from a team as code owners May 8, 2020 23:34
@mcous mcous requested review from shlokamin and removed request for a team May 8, 2020 23:34
@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #5637 into edge will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #5637      +/-   ##
==========================================
+ Coverage   64.97%   65.02%   +0.05%     
==========================================
  Files        1099     1099              
  Lines       30637    30686      +49     
==========================================
+ Hits        19905    19954      +49     
  Misses      10732    10732              
Impacted Files Coverage Δ
...otSettings/SelectNetwork/ConnectModal/FormModal.js 100.00% <ø> (ø)
...obotSettings/SelectNetwork/ConnectModal/FormRow.js 100.00% <ø> (ø)
components/src/controls/ControlSection.js 100.00% <ø> (ø)
components/src/structure/Card.js 100.00% <ø> (ø)
components/src/primitives/Flex.js 100.00% <100.00%> (ø)
components/src/primitives/Text.js 100.00% <100.00%> (ø)
components/src/styles/colors.js 100.00% <100.00%> (ø)
components/src/styles/typography.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 330acc0...f9e7702. Read the comment docs.

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

This seems really cool and looks solid, I like the methodology behind this. Thanks, @mcous

Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

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

💅 🖥️
I really like this @mcous. This is a step towards making our lives a little easier in overriding the complibs more complicated components' default styles. This might be a good candidate for a lunch and learn with FE devs + UX to help us align and adopt moving forward!

@mcous mcous merged commit b1b318e into edge May 11, 2020
@mcous mcous deleted the components_primitives branch May 11, 2020 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
components Affects the `components` project JS Tech Debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants