Theme-ui: 'Text' component to use 'p' or span as default

[isaac-martin]

Have just picked this library up on a project, and so far so good. Except I worry about the fact that the default HTML element that is used for the <Text> primitive is a div - would there be pushback for me to PR to swap this to a p or span and add an as prop to change the base renderer?

[lachlanjc]

We have the as prop already! But yeah, a <p> tag by default would be awesome, I don’t know why we haven’t changed that already.

[atanasster]

Changing it to a default <p />will be a breaking change/different margin defaults, right?

[lachlanjc]

Yep, definitely a breaking change. Also if you're styling children via selectors etc. Would make more semantic HTML by default though.

[atanasster]

I think it would be correct for Text to have been a <span /> and a separate Paragraph component

[lachlanjc]

Nonetheless, span is inline by default while p is block by default, so definitely still a breaking change

[isaac-martin]

Changing it to a default <p />will be a breaking change/different margin defaults, right?

We _may_ be able to default the margins / spacings to be the same as div - maybe making it a span so that it is inline by default.

[atanasster]

I dont know if we should introduce a breaking change, but if we do, i vote for <span /> :)

[flo-sch]

Might be just me, but I almost always use <Text as="p" /> for actual HTML paragraphs, and very rarely need an inline element

Both the breaking-change and having another "Paragraph" component defaulting to a "p" sound good to me

[atanasster]

@flo-sch - would you want to contribute a <Paragraph /> component?

[hasparus]

@flo-sch @atanasster do we need a Paragraph component? I mean, would it be different to <Text as="p" />?

[atanasster]

@hasparus i think we need a Paragraph - it will have its own theme key.
Most component libraries i have used before theme-ui do have a Text, which is a span and a separate Paragraph component, so it will be an easier transition as well.

[lachlanjc]

Interesting. Yeah, I very occasionally use Text inline but it's almost always a paragraph, & I sometimes forget to include the as. I'm in favor of making Text return a p by default, since that's what I've always expected anyway, but the theme keys are a totally valid reason for a separate component too.

[atanasster]

I am just afraid we can run into really hard to catch breaking changes with changing Text to a p.
Adding a Paragraph seems to me to have a few more benefits

• non breaking
• avoids re-typing all the time as="p"
• has its own theme key
• seems more friendly for people coming from other ui libraries.

what do you guys think?

[flo-sch]

I mean, would it be different to <Text as="p" />?

I am not actually 100% sure, since today, <Text /> is a div with margin: 0 (inherited from Box)

I missed the fact that Box sets a margin, which in our case might invalidate the issue of the default margin applied to an HTML <p>, or am I missing something?

Then if we add a new Paragraph component, it would mainly bring a new themeKey for texts (which indeed does not seem to add a lot of value, since we can already define variants inside theme.text)

[flo-sch]

I have nothing against a new component neither per say, I would personally be more tempted to look for a <Paragraph /> rather than a <Text />.

(However I would personally find span as default for <Text /> to be a rather annoying BC)

[hasparus]

Okay, I don't see another solution to this.

https://www.strawpoll.me/33187129

I'm biased towards no-paragraph & Text as span, because that's how Braid does it, but I don't care strongly about it.

cc @dcastil

[atanasster]

@hasparus - that polling is great !!!

Can you please add also Text stays "div" underneath, add Paragraph so we dont have any compatibility issues.

Also not feeling too strong about it.

[hasparus]

Can you please add also Text stays "div" underneath, add Paragraph so we dont have any compatibility issues.

The poll options are immutable already.

[atanasster]

np - i voted for adding Paragraph and change to span then :)

[isaac-martin]

If we are not defaulting the Text component to resolve to a semantic HTML text element in this library we are introducing bad habits for people on the web - particularly those who may just be starting out with web dev and do not know any better.
Especially considering how theme-ui is tightly coupled to Gatsby a tool very popular with people relatively new to web dev.

I _think_ with a codemod and a breaking change we could do this.

The codemod could find every usecase of <Text> without an as and replace it with <Text as="div" /> We could probably type that to be deprecated or warn as well

@flo-sch you mention "However I would personally find span as default for to be a rather annoying BC" - what would be annoying about that for you? (Not attacking just curious in case I have missed something)

[dcastil]

<p> elements have semantics attached to them. E.g. you can't nest <p> elements in each other. They also have default styles attached, like a margin-top and margin-bottom. These things might make it harder to use.

[atanasster]

We have some results, but very tight :)

• change <Text /> to p - 4 votes
• change <Text /> to span - total of 5 votes (3 of them add <Paragraph />, 2 do not add any new components)

[flo-sch]

If we are not defaulting the Text component to resolve to a semantic HTML text element in this library we are introducing bad habits for people on the web - particularly those who may just be starting out with web dev and do not know any better.

100% agree with that, div is not optimal as default for a component meant to display text blocks (the way I use <Text /> personally anyway)

@flo-sch you mention "However I would personally find span as default for to be a rather annoying BC" - what would be annoying about that for you? (Not attacking just curious in case I have missed something)

The display: inline property that span elements get by default (unless I missed a display: block being added to Text or Box somewhere? 🤔)

I almost always use <Text as="p" /> anyway but sometimes miss it, with html5 elements and CSS flexbox positioning, I never use span elements anymore

Anyway, I am clearly in favor of a styled component defaulting to <p>, wether it's called <Paragraph /> or <Text /> :)

[hasparus]

We have some results, but very tight :)

change to p - 4 votes
change to span - total of 5 votes (3 of them add , 2 do not add any new components)

We agree that the div is a horrible default, let's fix it. I believe we can leave the choice of the solution to the person making the PR and writing the changelog :)

[atanasster]

@hasparus - excellent decision, in a close call I also always leave it to the person doing the work(PR) :)

Any takers for this feature? I will have some free time in a day or two if no one jumps in.

[flo-sch]

I can take that, sounds doable for me :)

[goldins]

FWIW Text is also a global: new window.Text('hello world'). Unsure if it matters but some consumers might be confused.

[hasparus]

FWIW Text is also a global: new window.Text('hello world').

It's so annoying to auto-import it because of this -.- That's a point in favor of Paragraph.

[flo-sch]

The components in master branch are still written in JS though (not TS), is it better if I base my PR on a different branch, or work on those?

[atanasster]

@flo-sch - imo work on those in master - and when the components are moved to ts, this code will also be updated. The current ts PRs are having conflicts and probably need a bit more work. @hasparus

[hasparus]

@atanasster @flo-sch the components on master are official. Just update that index.d.ts and you're good.

[flo-sch]

Completely missed the index.d.ts 🤦, will add that to the PR