patternfly-react icon indicating copy to clipboard operation
patternfly-react copied to clipboard

Add support for classes on TextContent children

Open mcoker opened this issue 1 year ago • 1 comments

follow up for core issue https://github.com/patternfly/patternfly/pull/6511

A quick overview of the change:

The current <TextContent> and <Text> component(s) basically work this way:

  • <TextContent> serves as a wrapper and styles/formats simple HTML elements within it.
  • <Text> renders simple HTML elements.
  • Exhibit A. Browser rendered content -> react code -> simple HTML elements rendered in the content wrapper in dev tools
    • Screenshot 2024-04-09 at 10 50 44 AM

This is cool, but comes with some issues.

  • Since the markup has to be in the <TextContent> wrapper, you can't really sprinkle use of formatted content around the page. Doing so would require the <TextContent> wrapper for everywhere it's used, and you don't get the intended spacing/margin unless all of the things you want to have space/margin are in the <TextContent> wrapper, and not everything plays nicely in the wrapper, which is the next point.
  • Since <TextContent> styles its children based off of their element type (<p>, <ul>, <h1>, etc), if you put another component in it (eg, <List>), <TextContent> will apply its styling to any of the elements it supports in the component you've included, which 90% of the time will probably break the component styling.

The update in core leaves the existing basic styling as it is. Elements are still styled via HTML elements as children of <TextContent> - .pf-v6-c-content p, .pf-v6-c-content h1, etc. But it extends all of the existing styles to also apply to a class for the child element used (.pf-v6-c-content--p, .pf-v6-c-content--h1, etc) and that class can be used inside or outside of <TextContent>.

My proposal would be to:

  • Update <Text>, <TextList>, and <TextItem> to simply include the new class appropriate for the element
    • <Text component={TextVariants.p}> renders <p class="pf-v6-c-content--p">
  • Make <TextContent> optional in the docs
  • Clarify in the docs that <Text>, <TextList>, and <TextItem> can be used on their own outside of <TextContent>, and if there is styling for something like :is(.pf-v6-c-content p, .pf-v6-c-content--p):not(:last-child) { // a bottom margin }, a paragraph will now have a bottom margin if it comes before anything else (like a <Table> for example)
  • Add a new example with no <TextContent> wrapper. Or update all of the examples to remove the <TextContent> wrapper, and include a simple example that uses <TextContent> - I think would be my preference, I imagine this is the most common use case.

mcoker avatar Apr 09 '24 16:04 mcoker

Depends on #10314

kmcfaul avatar May 07 '24 15:05 kmcfaul