Skip to content
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

feat: Add option for line-wrapping to prevent horizontal scrolling #20

Merged
merged 45 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
59ecbce
feat: add option for line-wrapping to prevent horizontal scrolling
akoreman Feb 24, 2024
9097cfc
Merge branch 'cloudscape-design:main' into line_wrap
akoreman Feb 24, 2024
6cd99c4
chore: remove accidentally commited npm script
akoreman Feb 25, 2024
b40b2b0
fix: make line numbers unselectable
akoreman Feb 25, 2024
6b9b6e7
chore: replace Ace text highlight rules with in-repo function
akoreman Feb 25, 2024
c9dd827
Merge branch 'main' into line_wrap
Mar 12, 2024
81bc51e
chore: Fix minimum-height and actions absolute positioning
Mar 12, 2024
4cd5711
fix: Fix code indentation by setting white-space to pre
Mar 12, 2024
1cdc45f
chore: Fix code color and lines Box variant
Mar 12, 2024
078fd46
chore: Add multi-line test
Mar 12, 2024
d44f75b
Merge branch 'main' into line_wrap
Mar 12, 2024
fb13d49
chore: Increase size-limit by 8 bytes
Mar 12, 2024
6da84dc
chore: Use logical properties
Mar 12, 2024
0658716
chore: Update findContent() test to be backward compatible
Mar 12, 2024
73a2b33
Give <table> role="presentation"
gethinwebster Mar 20, 2024
e1982b3
feat: add option for line-wrapping to prevent horizontal scrolling
akoreman Feb 24, 2024
a6277a0
chore: remove accidentally commited npm script
akoreman Feb 25, 2024
d0550ce
fix: make line numbers unselectable
akoreman Feb 25, 2024
529aeb4
chore: replace Ace text highlight rules with in-repo function
akoreman Feb 25, 2024
6741164
chore: Fix minimum-height and actions absolute positioning
Mar 12, 2024
4dc0603
fix: Fix code indentation by setting white-space to pre
Mar 12, 2024
d746e42
chore: Fix code color and lines Box variant
Mar 12, 2024
d3f469e
chore: Add multi-line test
Mar 12, 2024
b015fba
chore: Increase size-limit by 8 bytes
Mar 12, 2024
b10273d
Give <table> role="presentation"
gethinwebster Mar 20, 2024
ceecca6
Increase size limit
gethinwebster Mar 20, 2024
f543ac5
Merge branch 'main' into line_wrap
akoreman Apr 2, 2024
323c76f
chore: increase size-limit
akoreman Apr 2, 2024
7929a08
Merge branch 'main' into line_wrap
akoreman Nov 14, 2024
ed856c0
Merge remote-tracking branch 'origin/line_wrap_upstream' into line_wrap
akoreman Nov 14, 2024
18ce8bb
small fixes after merging up to main
Nov 14, 2024
6e86b8d
small fix after merging up to main
Nov 14, 2024
e4c46c7
bump size limit
Nov 14, 2024
f300bd1
keep test util backwards compatible
Nov 14, 2024
5b7e6e6
small fixes from mistakes made while merging
Nov 14, 2024
713e49a
Merge branch 'main' into line_wrap
Dec 2, 2024
33411cc
revert highlight function type to return ReactNode
Dec 2, 2024
b0659b6
add syntax highlighting example line wrapping page
Dec 2, 2024
4fc29e2
small refactor
Dec 2, 2024
0dc2c7c
wrap text on pages in Box components
Dec 3, 2024
e4b2dae
update parameter jsdoc
Dec 3, 2024
4d69e81
update documentation snapshot
Dec 3, 2024
3b8ad86
update prop name to wrapLines
Dec 3, 2024
3de5396
add word break
Dec 16, 2024
df9f1b7
bump size
Dec 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
"size-limit": [
{
"path": "lib/components/index.js",
"limit": "0.5kb"
"limit": "508b"
},
{
"path": "lib/components/code-view/highlight/javascript.js",
Expand Down
3 changes: 3 additions & 0 deletions pages/code-view/simple.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export default function CodeViewPage() {
<ScreenshotArea>
<h1>Code View</h1>
<CodeView content={"Hello World"} />
<CodeView
content={`public class HelloWorld {\n public static void main(String[] args) {\n System.out.println("Hello, World!");\n }\n}`}
/>
</ScreenshotArea>
);
}
6 changes: 6 additions & 0 deletions pages/code-view/with-actions-button.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ export default function CodeViewPage() {
actions={<Button ariaLabel="Copy code" iconName="copy"></Button>}
content={`Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.`}
/>
<CodeView
lineNumbers={true}
lineWrapping={true}
actions={<Button ariaLabel="Copy code" iconName="copy"></Button>}
content={`Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.`}
/>
</SpaceBetween>
</ScreenshotArea>
);
Expand Down
40 changes: 40 additions & 0 deletions pages/code-view/with-line-wrapping.page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { SpaceBetween } from "@cloudscape-design/components";
import { CodeView } from "../../lib/components";
import { ScreenshotArea } from "../screenshot-area";
export default function CodeViewPage() {
return (
<ScreenshotArea>
<h1>Code View</h1>
<SpaceBetween direction="vertical" size="l">
No wrapping, no line numbers
Copy link
Member

Choose a reason for hiding this comment

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

Could you please update this page and also with-syntax-highlighting.page.tsx with wrapping text content inside SpaceBetween in some container, e.g. a Box? That is to satisfy the guidelines: https://cloudscape.aws.dev/components/space-between/?tabId=api

Currently, there are React warnings in these pages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah yeah good point, fixed in new commit

<CodeView
content={`Hello this is a short line.\nHello this is a long line. Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.\nHello this is another short line.`}
/>
No wrapping, line numbers
<CodeView
lineNumbers={true}
content={`Hello this is a short line.\nHello this is a long line. Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.\nHello this is another short line.`}
/>
Wrapping, no line numbers
<CodeView
lineWrapping={true}
content={`Hello this is a short line.\nHello this is a long line. Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.\nHello this is another short line.`}
/>
Wrapping, line numbers
<CodeView
lineWrapping={true}
lineNumbers={true}
content={`Hello this is a short line.\nHello this is a long line. Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.\nHello this is another short line.`}
/>
<CodeView
lineWrapping={true}
lineNumbers={true}
content={`public class HelloWorld {\n public static void main(String[] args) {\n System.out.println("Hello, World!");\n }\n}`}
/>
</SpaceBetween>
</ScreenshotArea>
);
}
10 changes: 9 additions & 1 deletion src/__tests__/__snapshots__/documenter.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ exports[`definition for code-view matches the snapshot > code-view 1`] = `
"description": "A function to perform custom syntax highlighting.",
"name": "highlight",
"optional": true,
"type": "(code: string) => React.ReactNode",
"type": "(code: string) => ReactElement",
},
{
"description": "Controls the display of line numbers.
Expand All @@ -38,6 +38,14 @@ Defaults to \`false\`.
"optional": true,
"type": "boolean",
},
{
"description": "Controls lines wrap when overflowing on the right side.
Defaults to \`false\`.
",
"name": "lineWrapping",
"optional": true,
"type": "boolean",
},
],
"regions": [
{
Expand Down
47 changes: 42 additions & 5 deletions src/code-view/__tests__/code-view.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,24 @@ describe("CodeView", () => {
afterEach(() => {
cleanup();
});
test("correctly renders component content", () => {
test("correctly renders simple content", () => {
render(<CodeView content={"Hello World"}></CodeView>);
const wrapper = createWrapper()!.findCodeView();
expect(wrapper!.findContent().getElement().textContent).toBe("Hello World");
expect(wrapper!.findContent()[0].getElement()).toHaveTextContent("Hello World");
});

test("correctly renders multi line content", () => {
const content = `# Hello World\n\nThis is a markdown example.`;

render(<CodeView content={content}></CodeView>);
const wrapper = createWrapper()!.findCodeView();
const contentElements = wrapper!.findContent();
expect(contentElements.length).toEqual(3);
const renderedContent = contentElements
.map((element) => element.getElement().textContent)
.join("")
.trim();
expect(renderedContent).toBe(content);
});

test("correctly renders copy button slot", () => {
Expand All @@ -26,7 +40,9 @@ describe("CodeView", () => {
test("correctly renders line numbers", () => {
render(<CodeView content={`Hello\nWorld\n!`} lineNumbers={true}></CodeView>);
const wrapper = createWrapper()!.findCodeView();
expect(wrapper!.findByClassName(styles["line-numbers"])!.getElement()).toHaveTextContent("123");
expect(wrapper!.findAllByClassName(styles["line-number"])[0]!.getElement()).toHaveTextContent("1");
expect(wrapper!.findAllByClassName(styles["line-number"])[1]!.getElement()).toHaveTextContent("2");
expect(wrapper!.findAllByClassName(styles["line-number"])[2]!.getElement()).toHaveTextContent("3");
});

test("correctly renders aria-label", () => {
Expand Down Expand Up @@ -57,18 +73,39 @@ describe("CodeView", () => {
></CodeView>
);
const wrapper = createWrapper().findCodeView()!;
expect(wrapper!.findContent().getElement().innerHTML).toContain('class="tokenized"');
expect(wrapper!.findContent()[0].getElement().innerHTML).toContain("tokenized");
});

test("correctly tokenizes content if highlight is set to language rules", () => {
render(<CodeView content={'const hello: string = "world";'} highlight={typescriptHighlightRules}></CodeView>);
const wrapper = createWrapper().findCodeView()!;
const element = wrapper!.findContent().getElement();
const element = wrapper!.findContent()[0].getElement();

// Check that the content is tokenized following typescript rules.
expect(getByText(element, "const")).toHaveClass("ace_type");
expect(getByText(element, "hello")).toHaveClass("ace_identifier");
expect(getByText(element, "string")).toHaveClass("ace_type");
expect(getByText(element, '"world"')).toHaveClass("ace_string");
});

test("sets nowrap class to line if linesWrapping undefined", () => {
render(<CodeView content={"Hello World"}></CodeView>);
const wrapper = createWrapper().findCodeView()!;
const element = wrapper!.findContent()[0].getElement();
expect(element.outerHTML).toContain("code-line-nowrap");
});

test("sets nowrap class to line if linesWrapping false", () => {
render(<CodeView lineWrapping={false} content={"Hello World"}></CodeView>);
const wrapper = createWrapper().findCodeView()!;
const element = wrapper!.findContent()[0].getElement();
expect(element.outerHTML).toContain("code-line-nowrap");
});

test("sets wrap class to line if linesWrapping true", () => {
render(<CodeView lineWrapping={true} content={"Hello World"}></CodeView>);
const wrapper = createWrapper().findCodeView()!;
const element = wrapper!.findContent()[0].getElement();
expect(element.outerHTML).toContain("code-line-wrap");
});
});
4 changes: 3 additions & 1 deletion src/code-view/highlight/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { Fragment } from "react";
import "ace-code/styles/theme/cloud_editor.css";
import "ace-code/styles/theme/cloud_editor_dark.css";

export function createHighlight(rules: Ace.HighlightRules) {
type CreateHighlightType = (code: string) => React.ReactElement;

export function createHighlight(rules: Ace.HighlightRules): CreateHighlightType {
return (code: string) => {
const tokens = tokenize(code, rules);
return (
Expand Down
9 changes: 8 additions & 1 deletion src/code-view/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ export interface CodeViewProps {
*/
lineNumbers?: boolean;

/**
* Controls lines wrap when overflowing on the right side.
*
* Defaults to `false`.
*/
lineWrapping?: boolean;

/**
* An optional slot to display a button to enable users to perform actions, such as copy or download the code snippet.
*
Expand All @@ -34,5 +41,5 @@ export interface CodeViewProps {
* A function to perform custom syntax highlighting.
*
*/
highlight?: (code: string) => React.ReactNode;
highlight?: (code: string) => React.ReactElement;
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Member

Choose a reason for hiding this comment

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

I think it got it: we ensure the type is ReactElement so that Children.map() can be used. However, this change is not backward-compatible because the ReactElement type is more narrow than ReactNode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah true that ReactElement is more narrow than ReactNode, I've updated the PR to try to work around it by creating a ReactElement based on the ReactNode given to us.

}
87 changes: 60 additions & 27 deletions src/code-view/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,69 +4,102 @@ import { useCurrentMode } from "@cloudscape-design/component-toolkit/internal";
import Box from "@cloudscape-design/components/box";
import clsx from "clsx";
import { useRef } from "react";
import { Children } from "react";
import { InternalBaseComponentProps, getBaseProps } from "../internal/base-component/use-base-component";
import { CodeViewProps } from "./interfaces";
import styles from "./styles.css.js";

const ACE_CLASSES = { light: "ace-cloud_editor", dark: "ace-cloud_editor_dark" };

function getLineNumbers(content: string) {
return content.split("\n").map((_, n) => n + 1);
}

type InternalCodeViewProps = CodeViewProps & InternalBaseComponentProps;

// Breaks down the input code for non-highlighted code-view into React
// Elements similar to how a highlight function would do.
const textHighlight = (code: string) => {
const lines = code.split("\n");
return (
<span>
{lines.map((line, lineIndex) => (
<span key={lineIndex}>
{line}
{"\n"}
</span>
))}
</span>
);
};

export function InternalCodeView({
content,
actions,
lineNumbers,
lineWrapping,
highlight,
ariaLabel,
ariaLabelledby,
__internalRootRef = null,
...props
}: InternalCodeViewProps) {
const code = highlight ? highlight(content) : <span>{content}</span>;
const baseProps = getBaseProps(props);
const preRef = useRef<HTMLPreElement>(null);
const darkMode = useCurrentMode(preRef) === "dark";

const regionProps = ariaLabel || ariaLabelledby ? { role: "region" } : {};

// Create tokenized elements of the content.
const code = highlight ? highlight(content) : textHighlight(content);

return (
<div
className={styles.root}
className={clsx(darkMode ? ACE_CLASSES.dark : ACE_CLASSES.light, styles.root)}
{...regionProps}
{...baseProps}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledby}
ref={__internalRootRef}
>
<div className={clsx(lineNumbers && styles["root-with-numbers"], actions && styles["root-with-actions"])}>
<Box color="text-status-inactive" fontSize="body-m">
{lineNumbers && (
<div className={styles["line-numbers"]} aria-hidden={true}>
{getLineNumbers(content).map((number) => (
<span key={number}>{number}</span>
))}
</div>
)}
</Box>
<pre
ref={preRef}
<div className={styles["scroll-container"]}>
<table
Copy link
Member

Choose a reason for hiding this comment

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

is it necessary for this to be semantically a table? or is this more just to get table layout, in which case could we do that purely with CSS? (thinking about a11y, and whether it makes sense for this to be announced as a "table")

Copy link
Contributor

Choose a reason for hiding this comment

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

The primary reason for choosing a table here is to align line numbers with their corresponding lines of code, now that we are introducing line-wrapping. Let us consult with A11Y to double-check.

className={clsx(
darkMode ? ACE_CLASSES.dark : ACE_CLASSES.light,
styles.code,
lineNumbers && styles["code-with-line-numbers"],
actions && styles["code-with-actions"]
styles["code-table"],
actions && styles["code-table-with-actions"],
lineWrapping && styles["code-table-with-line-wrapping"]
)}
>
<Box color="inherit" variant="code" fontSize="body-m">
{code}
</Box>
</pre>
{actions && <div className={styles.actions}>{actions}</div>}
<colgroup>
<col style={{ width: 1 } /* shrink to fit content */} />
<col style={{ width: "auto" }} />
</colgroup>
<tbody>
{Children.map(code.props.children, (child, index) => {
return (
<tr key={index}>
{lineNumbers && (
<td className={clsx(styles["line-number"], styles.unselectable)} aria-hidden={true}>
<Box variant="code" color="text-status-inactive" fontSize="body-m">
{index + 1}
</Box>
</td>
)}
<td className={styles["code-line"]}>
<Box variant="code" fontSize="body-m">
<span
className={clsx(
code.props.className,
lineWrapping ? styles["code-line-wrap"] : styles["code-line-nowrap"]
)}
>
{child}
</span>
</Box>
</td>
</tr>
);
})}
</tbody>
</table>
</div>
{actions && <div className={styles.actions}>{actions}</div>}
</div>
);
}
Loading
Loading