feat(code-block): add wrapCode option #991
Conversation
🦋 Changeset detectedLatest commit: d414d01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
zchsh
added
the
release:canary
📦 Canary Packages PublishedLatest commit: d414d01 Published 1 packages@hashicorp/react-code-block@6.4.0-canary-20230809002333 |
| code={`A line that goes on for a very long time so that it overflows the container in which it is located, which might be a pretty wide container.\nThis is a second line of code.\nAnd a third line.\nAnd another line, this is the fourth line.`} | ||
| /> | ||
|
|
||
| #### Wrap Code |
There was a problem hiding this comment.
Added a section to docs.mdx to demo the new behaviour.
| highlight?: boolean | ||
| highlight?: string |
There was a problem hiding this comment.
I converted the code-lines partial to .tsx in this process, this is a type error unrelated to code wrapping that came up.
There was a problem hiding this comment.
This file is likely easier to view in "side by side" mode, as the corresponding JSX has been significantly refactored.
| // When the floating copy button is present, we add padding to many lines | ||
| const padRight = Boolean(hasFloatingCopyButton) | ||
|
|
||
| if (wrapCode) { |
There was a problem hiding this comment.
I've refactored this component fairly significantly, which felt necessary in order to make the wrapCode option feasible.
For some context, there are fundamentally different layout needs with wrapCode compared to without:
- When allowing overflow, we want the lines of code to scroll together, but we want the line numbers for each line to stay in a fixed position, like a frozen spreadsheet column. This necessitates a two-column layout at the top level, with the line numbers in one column, and the line contents in a separate adjacent column with scrollable content.
- When wrapping code, a single line will take up more vertical space when it wraps. The two-column layout needed for overflowing code is no longer usable - the only approach I can imagine would be to somehow detect wrapping in each line in the "lines" columns and constantly update the corresponding line number contents in the "numbers" column to contain an identical number of newlines. It seemed much easier to rethink this use case as an alternate layout of the same atomic components, with a single column where each "row" is a flex layout that contains both the line number and the line contents.
To achieve these varied layouts, the previous CodeLines component was first refactored into smaller composable components, and then here in this conditional, we compose those smaller components into the appropriate layouts depending on the incoming wrapCode boolean.
|
@dstaley Tagged ya here since while this PR and the |
| import s from './code-lines.module.css' | ||
|
|
||
| interface CodeLinesProps { | ||
| code: string | ReactElement |
There was a problem hiding this comment.
Just curious: is there a specific reason you went with ReactElement? I think code: ReactNode would be the more accurate type since that's inclusive of string.
There was a problem hiding this comment.
Great point! Intent was that we might want string | ReactElement as it gets a little more specific about code not being any of the other possible types under ReactNode:
type ReactNode = ReactElement | string | number | ReactFragment | ReactPortal | boolean | null | undefined;Open to changing it if that doesn't feel quite right though 🙇♂️
| wrapCode, | ||
| }: CodeLinesProps) { | ||
| // Parse out an array of integers representing which lines to highlight | ||
| const highlightedLines = parseHighlightedLines(highlight) as number[] |
There was a problem hiding this comment.
Does this need to be cast?
There was a problem hiding this comment.
For now yes, as otherwise it seems the inferred type of highlightedLines ends up being never[], and we get a type error later at highlightedLines.indexOf(number) !== -1.
Ideally I'd have jumped in and converted parseHighlightedLines to TypeScript as well, but I didn't quite have the energy 😅
🎟️ Asana Task
🔍 Preview Link
This PR updates the
code-blockto add support for awrapCodebooleanoption.When enabled, the
wrapCodeoption causes long lines of code within a code block to wrap to new lines, rather than create horizontal overflow within the code block.🎨 Screenshots
options.wrapCode={true}💻 Demo
2023-08-14-wrap-code-demo.mp4
🧪 Testing
wrapCodeexample at the start of theFeatures & Guidelinessection should look and function as expected, across viewport sizesPR Checklist 🚀
Items in this checklist may not may not apply to your PR, but please consider each item carefully.