feat(misc): Replace Text, TextContent, TextList and TextListItem with Content - PART 2#10643
Conversation
|
Preview: https://patternfly-react-pr-10643.surge.sh A11y report: https://patternfly-react-pr-10643-a11y.surge.sh |
2fb1edc to
d72cde6
Compare
4ce7667 to
55d6063
Compare
thatblindgeye
left a comment
There was a problem hiding this comment.
Few changes below.
Also this isn't a blocker for this PR, but would be worth post v6 to clean up usage of Content for consistency. Right now we're mixing and matching between <Content component="h1">... and just using a native HTML <h1>.... Even if both are valid, might be better to stick to one in our examples/codebase, except any examples that are intended to show "you can also use native element inside of Content".
packages/react-core/src/components/Content/examples/ContentPlainList.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/components/Content/examples/ContentPlainList.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/components/Content/examples/ContentOrderedList.tsx
Outdated
Show resolved
Hide resolved
| Content with a component of type "p" still renders the same when wrapped with a TextContent. | ||
| </Content> | ||
| <p>If located within a wrapping TextContent, html elements are styled as well!</p> |
There was a problem hiding this comment.
Verbiage may need to be updated here, re: "...TextContent" still being used
There was a problem hiding this comment.
@mcoker Is this example even needed. It is a new one?
packages/react-core/src/demos/DescriptionList/examples/DescriptionListBasic.tsx
Outdated
Show resolved
Hide resolved
| --- | ||
|
|
||
| The `<Text>` component provides simple, built-in styling for putting common blocks of HTML elements together. It establishes the block of content and styling within it for the elements listed in the `component` property(`h1` through `h6`, `p`, `a`, `small`, `blockquote`, and `pre`), as well as the text component suite `<TextList>`, and `<TextListItem>`. `TextContent` may be used as a container for the text components, but nesting them inside `<TextContent>` is not required. | ||
| The `<Content>` component provides simple, built-in styling for putting common blocks of HTML elements together. It establishes the block of content and styling within it for the elements listed in the `component` property(`h1` through `h6`, `hr`, `p`, `a`, `small`, `blockquote`, and `pre`). |
There was a problem hiding this comment.
| The `<Content>` component provides simple, built-in styling for putting common blocks of HTML elements together. It establishes the block of content and styling within it for the elements listed in the `component` property(`h1` through `h6`, `hr`, `p`, `a`, `small`, `blockquote`, and `pre`). | |
| The `<Content>` component allows you to establish a block of HTML content and apply simple, built-in styling. `<Content>` can be used for any element supported by the `component` property (including `h1` through `h6`, `hr`, `p`, `a`, `small`, `blockquote`, and `pre`). |
| You cannot nest other components within `<Content>`, and doing so can cause styling overrides or other conflicts. Instead, you can use the `<Content>` component's properties to achieve the same results. | ||
|
|
||
| For example, rather than nesting the `<List>` and `<Title>` components within `<Text>`, you should pass `component="h1"` into the `<TextList>` and `<Text>` components. Similarly, when you need to add a divider , rather than passing in a separate `<Divider>` component, you should utilize the `hr` property that `<Text>` supports, which will be styled as a divider. | ||
| For example, rather than nesting the `<List>` and `<Title>` components within `<Content>`, you should pass `component="h1"` into the `<Content>` component. Similarly, when you need to add a divider , rather than passing in a separate `<Divider>` component, you should utilize the `hr` property that `<Content>` supports, which will be styled as a divider. |
There was a problem hiding this comment.
| For example, rather than nesting the `<List>` and `<Title>` components within `<Content>`, you should pass `component="h1"` into the `<Content>` component. Similarly, when you need to add a divider , rather than passing in a separate `<Divider>` component, you should utilize the `hr` property that `<Content>` supports, which will be styled as a divider. | |
| For example, to create a level 1 heading, you should pass `component="h1"` to `<Content>`, instead of nesting a `<Title>` component within `<Content>`. Similarly, when you need to add a divider to a page, you should utilize the `hr` property of `<Content>` (which is styled as a divider), rather than using a separate `<Divider>` component. |
maybe I'm misunderstanding, but is mentioning <List> relevant for this example? It doesn't intrinsically apply h1 does it?
| ``` | ||
|
|
||
| Text components such as Text, TextList, TextListItem can be placed within a TextContent to provide styling for html elements, and additional styling options applied to the children. | ||
| Html elements wrapped by `<Content>` are styled by the Content component. |
There was a problem hiding this comment.
| Html elements wrapped by `<Content>` are styled by the Content component. | |
| HTML elements wrapped by `<Content>` are styled by the content component. |
Maybe it would make more sense to me if I could load the website preview, but I'm not fully understanding the purpose of this line. Is this different than what we've already stated above?
| <Content> | ||
| <h1>Main title</h1> | ||
| <p> | ||
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> |
There was a problem hiding this comment.
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> | |
| Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br /> |
this aligns with our higher level typography rules/docs
| <Content> | ||
| <h1>Main title</h1> | ||
| <p> | ||
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> |
There was a problem hiding this comment.
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> | |
| Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br /> |
| <Content> | ||
| <h1>Main title</h1> | ||
| <p> | ||
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> |
There was a problem hiding this comment.
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> | |
| Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br /> |
| <Content> | ||
| <h1>Main title</h1> | ||
| <p> | ||
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> |
There was a problem hiding this comment.
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> | |
| Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br /> |
| <Content> | ||
| <h1>Main title</h1> | ||
| <p> | ||
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> |
There was a problem hiding this comment.
| Body text should be Overpass Regular at 16px. It should have leading of 24px because <br /> | |
| Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br /> |
| </TextContent> | ||
| <Content> | ||
| <h1>Projects</h1> | ||
| <p>Click any project card to view Tabs within Modals.</p> |
There was a problem hiding this comment.
| <p>Click any project card to view Tabs within Modals.</p> | |
| <p>Click any project card to view tabs within modals.</p> |
| <Content> | ||
| <h1>Main title</h1> | ||
| <p> | ||
| Body text should be Overpass Regular at 16px.It should have leading of 24px because <br /> |
There was a problem hiding this comment.
| Body text should be Overpass Regular at 16px.It should have leading of 24px because <br /> | |
| Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br /> |
Needed so that we can avoid temporary build problems caused by a chicken and egg situation between this repo and org.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #10395
This is a followup to Part 1 PR #10611 - wait for that PR to merge first.
This PR is breaking and the build will fail, because Text, TextContent, TextList and TextListItem have to be updated in
patternfly-orgso the build framework doesn't use these old components.Codemod PR: patternfly/pf-codemods#675
Org issue to replace Text: patternfly/patternfly-org#4087