Skip to content

[WIP] Prototype new Text components #146

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ const componentv2Loaders = [
"Clip",
"Link",
"QRCode",
"Slat"
"Slat",
"Text"
].map(fromComponentPathv2);

const App = () => {
Expand Down
43 changes: 43 additions & 0 deletions docs/Examples2/Text.example.purs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
module Lumi.Components2.Examples.Text where

import Prelude

import Lumi.Components (($$$))
import Lumi.Components.Example (example)
import Lumi.Components2.Box (box)
import Lumi.Components2.Text as T
import React.Basic (JSX, fragment)

docs :: JSX
docs =
box
$$$ [ example
$ fragment
$ [ T.sectionHeader $$$ "A tiny story"
, T.subsectionHeader $$$ "Hello!"
, T.paragraph
$$$ [ T.text
$ T._emphasis
$$$ "How are you? "
, T.text
$$$ "Hope you're doing "
, T.text
$ T._strong
$$$ "fine. "
, T.text
$ T._subtext
$ T._muted
$$$ "Yes, I do."
]
, T.paragraph
$$$ [ T.text $$$ "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam congue ligula et odio rutrum, eu imperdiet ante laoreet. Cras mollis faucibus urna, eu luctus ligula condimentum ut. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Vivamus et mi mattis, maximus urna id, luctus neque. Sed non lorem molestie nibh suscipit condimentum id quis enim. Nunc tortor elit, posuere eu metus sed, finibus sagittis est. Fusce dapibus lacus vitae augue vulputate, in convallis lectus congue. "
, T.text $ T._strong $$$ "Hi! "
, T.text $ T._subtext $$$ "Hey!"
]
, T.paragraph_
$$$ "Vestibulum eu arcu eget lectus interdum maximus feugiat sed velit. Integer ullamcorper urna quis cursus mattis. Nam vel hendrerit purus. Aliquam fringilla dictum nunc at ornare. Morbi ornare blandit tincidunt. Etiam sodales fringilla libero, vitae pulvinar sapien luctus ac. Proin condimentum vitae risus id vestibulum. Sed sed turpis leo. Quisque ligula leo, facilisis eget metus ullamcorper, aliquet mollis tortor. Donec purus metus, maximus rutrum nunc eget, rhoncus tempor erat. Sed efficitur tellus id velit ullamcorper, ut dignissim neque pretium. Sed id metus porta, efficitur est non, vulputate sapien."
, T.paragraph_
$ T._subtext
$$$ "Donec maximus commodo ipsum vel elementum. Sed at nunc dapibus, vulputate tellus eu, finibus ante. Nunc dolor ante, auctor et rutrum quis, sagittis ut nulla. Integer vel tempus ipsum, vel laoreet orci. Curabitur eu sem bibendum, rhoncus risus vel, tristique mauris. Cras lobortis elit sit amet quam semper pretium. Nullam fermentum ut velit ac cursus."
]
]
194 changes: 194 additions & 0 deletions src/Lumi/Components2/Text.purs
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
module Lumi.Components2.Text where
Copy link
Member

Choose a reason for hiding this comment

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

We might not want to expose mkHeaderComponent just for consistency. Do you think the Text constructor should be exposed (don't know, just curious)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, I forgot to constrain the exports 😅


import Prelude

import Data.Lens (Setter')
import Effect.Unsafe (unsafePerformEffect)
import Lumi.Components (LumiComponent, LumiProps, PropsModifier, lumiComponent, propsModifier)
import Lumi.Styles as S
import Lumi.Styles.Theme (LumiTheme(..), useTheme)
import React.Basic (JSX, ReactComponent)
import React.Basic.DOM as R
import React.Basic.Emotion as E
import React.Basic.Hooks as Hooks

-- Text

data Text = Text

type TextProps =
( component :: Text
, element :: TextElement
, content :: String
)

type TextElement = ReactComponent { children :: Array JSX, className :: String }

text :: LumiComponent TextProps
text =
unsafePerformEffect $ lumiComponent "Text" defaults \props -> Hooks.do
theme <- useTheme
pure $
E.element props.element
{ children: [ R.text props.content ]
, className: props.className
, css: defaultTextStyle <> props.css theme
}
where
defaults :: Record TextProps
defaults =
{ component: Text
, element: R.span'
, content: ""
}

defaultTextStyle =
S.css
{ fontSize: S.str "14px"
, lineHeight: S.str "17px"
, margin: S.str "0"
, padding: S.str "0"
Copy link
Member

Choose a reason for hiding this comment

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

I think we want Text to default to white-space: pre; and have Paragraph override that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe pre-wrap is safer?

Copy link
Member

Choose a reason for hiding this comment

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

I think you're right. I had been thinking we might want the base Text component to never wrap and Paragraph to contain the wrapping behavior/settings (like spacing/line-height which wouldn't be available on Text), but wrapping is just so much more common that that might be more annoying than helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Either way, Text and Paragraph should be setting it explicitly so we don't inherit unexpected values. We probably want to do the same with font-family, and pull both font-family and the sizing/line-height settings from the theme.

Copy link
Member

Choose a reason for hiding this comment

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

Side note, if we did make Text nowrap and Paragraph wrap, we'd probably have trouble doing that with the current api, where Paragraph's children are Texts (or other components with Text children) as the inner Text nowrap would be more specific. That probably means we have the same issue with other styles here.. Paragraph's settings might need to be marked important, unless you can think of a better option.

Copy link
Member

@maddie927 maddie927 May 6, 2020

Choose a reason for hiding this comment

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

I see paragraph_ is partially working around that here, but paragraph would be dependent on each child Text having its own sizing/line-height set. Hmm. I think.. Paragraph defaults and overrides should override all child Text defaults, but not child Text overrides, i.e.

paragraph
  $ emphasize
  $$$
    -- paragraphs are not emphasized by default but this one has a modifier
    -- but this local override still takes priority
    [ text $ nonEmphasized $$$ "hello"
    -- paragraphs have default text size and line height, but this local override
    -- would still take priority.
    -- this text inherits the paragraph emphasis setting
    , text $ fontSize 20 $$$ "goodbye"
    ]

Copy link
Member

Choose a reason for hiding this comment

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

These problems are kind of unique to Paragraph and Text

Copy link
Contributor Author

@arthurxavierx arthurxavierx May 8, 2020

Choose a reason for hiding this comment

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

Either way, Text and Paragraph should be setting it explicitly so we don't inherit unexpected values. We probably want to do the same with font-family, and pull both font-family and the sizing/line-height settings from the theme.

Yes, excellent points!

Side note, if we did make Text nowrap and Paragraph wrap

I don't think we should make Text nowrap. What would be the reason for doing that?

Paragraph defaults and overrides should override all child Text defaults, but not child Text overrides, i.e.

Maybe we could get around that by having all text defaults be inherit and setting those defaults globally? But then that'd kind of defeat the purpose of having LumiTheme :p

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should make Text nowrap. What would be the reason for doing that?

I think I agree, it was an idea I inherited from another api but it doesn't give us anything here.

Maybe we could get around that by having all text defaults be inherit and setting those defaults globally? But then that'd kind of defeat the purpose of having LumiTheme :p

Maybe.. it might be risky if there are non-lumi-components styles used anywhere on the page

}

type TextModifier = forall r. PropsModifier (component :: Text, element :: TextElement | r)

_body :: TextModifier
_body = propsModifier _{ element = R.span' :: TextElement }
Copy link
Member

Choose a reason for hiding this comment

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

We might not need to continue this underscore convention here, since the types are more descriptive (i.e. there isn't a special StyleModifier all the others depend on)


_strong :: TextModifier
_strong = propsModifier _{ element = R.strong' :: TextElement }

_emphasis :: TextModifier
_emphasis = propsModifier _{ element = R.em' :: TextElement }

_subtext :: TextModifier
_subtext =
propsModifier _{ element = R.small' :: TextElement }
<<< S.style_
( S.css
{ fontSize: S.str "12px"
, lineHeight: S.str "14px"
, marginBottom: S.str "6px"
}
)

_muted :: TextModifier
_muted =
S.style \(LumiTheme { colors }) ->
S.css { color: S.color colors.black1 }

-- Paragraph

type ParagraphProps =
( component :: Text
, element :: TextElement
, content :: Array JSX
)

paragraph_ :: LumiComponent TextProps
paragraph_ = paragraph <<< renderInnerText
where
renderInnerText :: Setter' (LumiProps ParagraphProps) (LumiProps TextProps)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this isn't being used with any lens functions, I feel like it might be easier to read if you expanded this Setter' type synonym.

renderInnerText f props =
let
props' = f $ props { content = "" }
in
props'
{ content =
[ text _
{ element = props'.element
, content = props'.content
, css = \_ -> S.css { fontSize: S.str "inherit", lineHeight: S.str "inherit" }
}
]
}

paragraph :: LumiComponent ParagraphProps
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure what's going on here. paragraph seems to ignore its element prop and always use p' instead, but paragraph_ doesn't?

paragraph =
unsafePerformEffect $ lumiComponent "Paragraph" defaults \props -> Hooks.do
theme <- useTheme
pure $
E.element R.p'
{ children: props.content
, className: props.className
, css: defaultParagraphStyle <> props.css theme
}
where
defaults :: Record ParagraphProps
defaults =
{ component: Text
, element: R.span'
, content: []
}

defaultParagraphStyle =
S.css
{ fontSize: S.str "14px"
, lineHeight: S.str "17px"
, margin: S.str "0"
, marginBottom: S.str "7px"
, padding: S.str "0"
}

-- Headers

type HeaderProps = (content :: String)

subsectionHeader :: LumiComponent HeaderProps
subsectionHeader = mkHeaderComponent R.h4'

sectionHeader :: LumiComponent HeaderProps
sectionHeader =
mkHeaderComponent R.h3'
<<< S.style_
( S.css
{ fontSize: S.str "20px"
, lineHeight: S.str "24px"
, marginBottom: S.str "10px"
}
)

title :: LumiComponent HeaderProps
title =
mkHeaderComponent R.h2'
<<< S.style_
( S.css
{ fontSize: S.str "24px"
, lineHeight: S.str "29px"
, marginBottom: S.str "12px"
}
)

mainHeader :: LumiComponent HeaderProps
mainHeader =
mkHeaderComponent R.h1'
<<< S.style_
( S.css
{ fontSize: S.str "30px"
, lineHeight: S.str "36px"
, marginBottom: S.str "15px"
}
)

mkHeaderComponent
:: TextElement
-> LumiComponent HeaderProps
mkHeaderComponent el =
unsafePerformEffect $ lumiComponent "Header" { content: "" } \props -> Hooks.do
theme <- useTheme
pure $
E.element el
{ children: [ R.text props.content ]
, className: props.className
, css: defaultHeaderStyle <> props.css theme
}
where
defaultHeaderStyle =
S.css
{ fontSize: S.str "17px"
, fontWeight: S.str "400"
, lineHeight: S.str "20px"
, padding: S.str "0"
, margin: S.str "0"
, marginBottom: S.str "9px"
}