-
Notifications
You must be signed in to change notification settings - Fork 27
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
Component: @phase2/outline-core-styled-text
#432
base: next
Are you sure you want to change the base?
Conversation
|
❌ Deploy Preview for outlinejs failed.
|
Important Auto Review SkippedDraft detected. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe update introduces the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
packages/components/outline-core-styled-text/packages.json
is excluded by:!**/*.json
packages/components/outline-core-styled-text/tsconfig.build.json
is excluded by:!**/*.json
Files selected for processing (12)
- packages/components/outline-core-styled-text/docs/outline-core-styled-text.mdx (1 hunks)
- packages/components/outline-core-styled-text/index.ts (1 hunks)
- packages/components/outline-core-styled-text/src/.ck.ck-content.global.css (1 hunks)
- packages/components/outline-core-styled-text/src/outline-core-styled-text.css (1 hunks)
- packages/components/outline-core-styled-text/src/outline-core-styled-text.lightdom.css (1 hunks)
- packages/components/outline-core-styled-text/src/outline-core-styled-text.shared.css (1 hunks)
- packages/components/outline-core-styled-text/src/outline-core-styled-text.ts (1 hunks)
- packages/components/outline-core-styled-text/src/sic-table.css (1 hunks)
- packages/components/outline-core-styled-text/src/utilities/check-file-type.ts (1 hunks)
- packages/components/outline-core-styled-text/src/utilities/external-link-text.ts (1 hunks)
- packages/components/outline-core-styled-text/src/utilities/tooltip.ts (1 hunks)
- packages/components/outline-core-styled-text/src/utilities/wrap-last-word.ts (1 hunks)
Additional comments: 13
packages/components/outline-core-styled-text/docs/outline-core-styled-text.mdx (13)
- 1-3: Imports are correctly defined at the top of the file, ensuring that necessary dependencies for the documentation are loaded. This includes the
html
function fromlit
and various Storybook components, as well as theOutlineCoreStyledText
component itself.- 5-14: The
<Meta>
tag is correctly configured with a title and parameters to hide the canvas view in the Storybook docs. This is a standard practice for Storybook documentation to control how the stories are displayed.- 40-57: The "Headings" story is well-constructed, demonstrating the use of various heading levels within the
OutlineCoreStyledText
component. This section effectively showcases how headings are styled when wrapped in the component.- 61-135: The "Basic Text" story provides a comprehensive demonstration of how different types of text and links are styled within the
OutlineCoreStyledText
component. It includes examples of paragraphs, links to different file types, and various text styles such as strong and italic paragraphs. This section is thorough and showcases the component's capabilities effectively.- 140-210: The "Disclaimer Text" story showcases the styling of disclaimer paragraphs within the
OutlineCoreStyledText
component. It includes examples of different paragraph styles and links, both internal and external. This section is well-detailed, providing clear examples of how disclaimer text can be styled.- 215-241: The "Block Quote" story demonstrates the styling of block quotes within the
OutlineCoreStyledText
component, including an icon for quotes. This section effectively showcases how block quotes are styled and includes an author attribution, providing a complete example of a styled block quote.- 245-263: The "Horizontal Rule" story demonstrates the use of a horizontal rule (
<hr />
) to separate content within theOutlineCoreStyledText
component. This is a simple yet effective demonstration of how horizontal rules can be styled and used within the component.- 267-317: The "Unordered List" story provides a detailed example of how unordered lists, including nested lists, are styled within the
OutlineCoreStyledText
component. This section effectively demonstrates the component's capability to style complex list structures.- 356-397: The "Ordered List" story demonstrates the styling of ordered lists, including nested lists with multiple levels, within the
OutlineCoreStyledText
component. This section is comprehensive and showcases the component's ability to handle complex list structures effectively.- 399-453: The "Table" story provides a detailed example of how tables are styled within the
OutlineCoreStyledText
component. This includes table headings, nested paragraph tags within table cells, and links. This section effectively demonstrates the component's capability to style tables in a consistent and accessible manner.- 455-482: The "Button and Link" story showcases the styling of buttons and links within the
OutlineCoreStyledText
component. It includes examples of standard links, external links, primary and secondary buttons, and call-to-action links. This section is comprehensive and effectively demonstrates the component's styling capabilities for interactive elements.- 484-505: The "Tooltip" story demonstrates the use of tooltips within the
OutlineCoreStyledText
component. It provides a clear example of how to create a tooltip using a link with atitle
attribute. This section effectively showcases the component's capability to enhance links with additional information in a user-friendly manner.- 507-652: The "Base Styled Text" story provides a comprehensive example of various text elements styled within the
OutlineCoreStyledText
component. It includes headings, paragraphs, lists, and links, showcasing the component's versatility in handling different types of content. This section is well-detailed and effectively demonstrates the component's styling capabilities.
# Overview | ||
|
||
These are the basic styles provided by the outline-core-styled-text component. | ||
`<outline-core-styled-text>` should wrap wysiwyg content. | ||
|
||
## Use In Components | ||
|
||
`<outline-core-styled-text>` will not inherit any css styles set on parent element. | ||
Components that contain slots that may use `<outline-core-styled-text>` will need to set | ||
css variables in component different values are needed. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The overview section provides a clear introduction to the OutlineCoreStyledText
component, explaining its purpose and how it should be used. However, it mentions that the component will not inherit CSS styles from parent elements and advises on setting CSS variables if different values are needed. This is crucial information for developers but could benefit from an example or further clarification on how to set these variables.
Consider adding a code snippet or a more detailed explanation on how to set CSS variables for components containing OutlineCoreStyledText
.
### CSS Variables | ||
|
||
- `--outline-core-body-text--font-size`: defaults to `--body-01-font-size` | ||
- `--outline-core-body-text--line-height`: defaults to `--body-01-font-size` | ||
- `--outline-core-body-text--margin-bottom`: defaults to `1em` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation correctly lists the CSS variables that can be used to customize the OutlineCoreStyledText
component. However, there's a typo in the description of --outline-core-body-text--line-height
, which incorrectly defaults to --body-01-font-size
instead of a line-height value.
- - `--outline-core-body-text--line-height`: defaults to `--body-01-font-size`
+ - `--outline-core-body-text--line-height`: defaults to `--body-01-line-height`
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
### CSS Variables | |
- `--outline-core-body-text--font-size`: defaults to `--body-01-font-size` | |
- `--outline-core-body-text--line-height`: defaults to `--body-01-font-size` | |
- `--outline-core-body-text--margin-bottom`: defaults to `1em` | |
### CSS Variables | |
- `--outline-core-body-text--font-size`: defaults to `--body-01-font-size` | |
- `--outline-core-body-text--line-height`: defaults to `--body-01-line-height` | |
- `--outline-core-body-text--margin-bottom`: defaults to `1em` |
## Two columns List (UL) | ||
|
||
<Canvas> | ||
<Story name="Two columns List"> | ||
{html` | ||
<outline-core-styled-text> | ||
<p> | ||
Sed fringilla mauris sit amet nibh. Ut varius tincidunt libero. Sed | ||
hendrerit. Proin magna. Nullam nulla eros, ultricies sit amet, nonummy | ||
id, imperdiet feugiat, pede. | ||
</p> | ||
<ul class="columns-2"> | ||
<li>This is the first item</li> | ||
<li>This is the second item</li> | ||
<li>This is the first child item</li> | ||
<li>This is the second child item</li> | ||
<li>This is the third item</li> | ||
<li>This is the first item</li> | ||
<li>This is the second item</li> | ||
<li>This is the first child item</li> | ||
<li>This is the second child item</li> | ||
<li>This is the third item</li> | ||
<li>This is the second child item</li> | ||
<li>This is the third item</li> | ||
</ul> | ||
<p> | ||
Sed fringilla mauris sit amet nibh. Ut varius tincidunt libero. Sed | ||
hendrerit. Proin magna. Nullam nulla eros, ultricies sit amet, nonummy | ||
id, imperdiet feugiat, pede. | ||
</p> | ||
</outline-core-styled-text> | ||
`} | ||
</Story> | ||
</Canvas> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Two columns List" story showcases how to create a two-column list using the OutlineCoreStyledText
component. This example is particularly useful for demonstrating more advanced styling capabilities within the component. However, it would benefit from a brief explanation or comment on how the columns-2
class is defined and used.
Consider adding a comment or documentation snippet explaining the columns-2
class and how it achieves the two-column layout.
packages/components/outline-core-styled-text/docs/outline-core-styled-text.mdx
Outdated
Show resolved
Hide resolved
Todos:
|
@phase2/outline-core-styled-text
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist
Summary by CodeRabbit
OutlineCoreStyledText
component for enhanced text styling and manipulation, including headings, paragraphs, block quotes, lists, tables, and links.