Skip to content

wip: Add prose macro#10161

Draft
devongovett wants to merge 2 commits into
mainfrom
prose
Draft

wip: Add prose macro#10161
devongovett wants to merge 2 commits into
mainfrom
prose

Conversation

@devongovett

Copy link
Copy Markdown
Member

Adds a prose() macro to S2 that styles semantic HTML elements within it, e.g. headings, paragraphs, code, etc. Useful for displaying markdown and other article content.

@github-actions github-actions Bot added the S2 label Jun 5, 2026
@rspbot

rspbot commented Jun 5, 2026

Copy link
Copy Markdown

textDecoration: 'underline',
transition: 'color 200ms'
},
':is(h1, h2, h3, h4, h5, h6, hr) + *': {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

gap between headings and following paragraphs should collapse to the smaller of the two (heading)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't css margins collapse already when siblings? I guess they collapse to the larger of the two, not the smaller... so do we want to have a different behaviour?

from looking at tailwind, I guess they are doing the same thing, so it's probably ok

@rspbot

rspbot commented Jun 5, 2026

Copy link
Copy Markdown

textDecoration: 'underline',
transition: 'color 200ms'
},
':is(h1, h2, h3, h4, h5, h6, hr) + *': {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't css margins collapse already when siblings? I guess they collapse to the larger of the two, not the smaller... so do we want to have a different behaviour?

from looking at tailwind, I guess they are doing the same thing, so it's probably ok

ol: {
listStyleType: 'decimal'
},
'li > p:last-child:not(:first-child)': {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is going to have a higher specificity than the others in this file, instead of 1 element or 0 0 1, this would be 0 2 2
(I am intentionally ignoring the leading classname "prose" for this, technically everything is one classname 0 1 0 higher than normal classes in our style macro)

again I see the tailwind prose has a bunch of selectors that violate that, so it's probably ok


let css = '';
for (let key in rules) {
let selector = key === '.prose' ? '.prose' : `.prose ${key}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how will prose work through mergeStyles? and will it also get a version appended to it?

return 'prose';
}

function font(value: keyof typeof fontSize) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

anything we need to do to keep this in sync with the spectrum-theme shorthand?
looks like the addition of --fs is really all that differs


const fontWeightBase = {
normal: '400',
normal: {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do you have to provide a default key in an object for these?

} as const;

const i18nFonts = {
export const i18nFonts = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

doesn't look like anything is using this?

export default meta;

export const Example = () => (
<article className={`${prose()} ${style({maxWidth: 800, marginX: 'auto'})}`}>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

so if someone does one of these? Since prose already attaches font('body') to itself

<article className={`${mergeStyles(prose(), style({maxWidth: 800, marginX: 'auto'}))`}>
<article className={`${prose()} ${style({maxWidth: 800, marginX: 'auto', font: 'title'})}`}>

@devongovett

Copy link
Copy Markdown
Member Author

You don't need to review wips @snowystinger 😉

@snowystinger

Copy link
Copy Markdown
Member

I know, but it's a good idea and I was just writing down any questions I had. Feel free to ignore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants