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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
35 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
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
35 changes: 35 additions & 0 deletions pages/code-view/with-line-wrapping.page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// 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
<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.`}
/>
</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
31 changes: 27 additions & 4 deletions src/code-view/__tests__/code-view.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe("CodeView", () => {
test("correctly renders component 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 copy button slot", () => {
Expand All @@ -26,7 +26,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 +59,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;
}
75 changes: 44 additions & 31 deletions src/code-view/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,71 +2,84 @@
// SPDX-License-Identifier: Apache-2.0
import { useCurrentMode } from "@cloudscape-design/component-toolkit/internal";
import Box from "@cloudscape-design/components/box";
import { TextHighlightRules } from "ace-code/src/mode/text_highlight_rules";
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 needed?

The original idea of this component to remove ace dependency from the bundle when syntax highlight is not used.

Copy link
Collaborator Author

@akoreman akoreman Feb 25, 2024

Choose a reason for hiding this comment

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

I needed something to go from string to ReactElement for non-highlighted code, for this draft/POC I used the texthighlight rules from Ace for that because it's quick to grab. I can write a simple function within this repo to do the same thing before merging if we decide this approach for line-wrapping looks good.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the info. Let's ensure it gets removed at the final stage

Copy link
Collaborator Author

@akoreman akoreman Feb 25, 2024

Choose a reason for hiding this comment

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

Removed it and replaced it with a function defined in the PR.

import clsx from "clsx";
import { useRef } from "react";
import { Children } from "react";
import { InternalBaseComponentProps, getBaseProps } from "../internal/base-component/use-base-component";
import { createHighlight } from "./highlight";
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;

const textHighlight = createHighlight(new TextHighlightRules());

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}
className={clsx(
darkMode ? ACE_CLASSES.dark : ACE_CLASSES.light,
styles.code,
lineNumbers && styles["code-with-line-numbers"],
actions && styles["code-with-actions"]
)}
>
<Box color="inherit" variant="code" fontSize="body-m">
{code}
</Box>
</pre>
{actions && <div className={styles.actions}>{actions}</div>}
</div>
<table className={clsx(styles["code-table"])}>
<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={styles["line-number"]} aria-hidden={true}>
<Box color="text-status-inactive" fontSize="body-m">
{index + 1}
</Box>
Copy link
Member

Choose a reason for hiding this comment

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

Changing the markup here breaks selection feature. Line numbers get added into selection.

For the reference you can check how it works on Github

Screen.Recording.2024-02-25.at.16.11.35.mov

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 that's a good catch, thanks. Added some CSS that should fix this.

</td>
)}
<td className={styles["code-line"]}>
<Box color="text-status-inactive" 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>
{actions && <div className={styles.actions}>{actions}</div>}
</div>
);
}
64 changes: 29 additions & 35 deletions src/code-view/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,57 +5,51 @@ $color-background-code-view-dark: #282c34;

.root {
position: relative;
&-with-numbers {
display: flex;
align-items: stretch;
}
}

.code {
:global(.awsui-dark-mode) &,
:global(.awsui-polaris-dark-mode) & {
background-color: $color-background-code-view-dark;
}
&-with-line-numbers {
border-start-start-radius: 0;
border-end-start-radius: 0;
flex: 1;
}
&-with-actions {
min-block-size: cs.$space-scaled-xxl;
padding-inline-end: calc(2 * cs.$space-static-xxxl);
align-items: center;
}
background-color: $color-background-code-view-light;
display: flex;
overflow-x: auto;
}

.code-table {
border-start-start-radius: cs.$border-radius-tiles;
border-start-end-radius: cs.$border-radius-tiles;
border-end-start-radius: cs.$border-radius-tiles;
border-end-end-radius: cs.$border-radius-tiles;
padding-block: cs.$space-static-xs;
padding-inline: cs.$space-static-xs;
margin-block: 0;
margin-inline: 0;
overflow: auto;
padding-top: cs.$space-static-xs;
rubencarvalho marked this conversation as resolved.
Show resolved Hide resolved
padding-bottom: cs.$space-static-xs;
table-layout: auto;
width: 100%;
border-spacing: 0;
}

.line-numbers {
.line-number {
border-right-color: cs.$color-border-divider-default;
:global(.awsui-dark-mode) &,
:global(.awsui-polaris-dark-mode) & {
background-color: $color-background-code-view-dark;
}
border-start-start-radius: cs.$border-radius-tiles;
border-end-start-radius: cs.$border-radius-tiles;
background-color: $color-background-code-view-light;
padding-block: cs.$space-static-xs;
padding-inline: cs.$space-static-xs;
display: flex;
flex-direction: column;
align-items: flex-end;
border-inline-end-width: 1px;
border-inline-end-style: solid;
border-inline-end-color: cs.$color-border-divider-default;
justify-content: center;
vertical-align: text-top;
position: sticky;
left: 0;
border-right-width: 1px;
border-right-style: solid;
padding-left: cs.$space-static-xs;
padding-right: cs.$space-static-xs;
}

.code-line {
padding-left: cs.$space-static-xs;
padding-right: cs.$space-static-xs;
&-wrap {
white-space: pre-wrap;
}
&-nowrap {
white-space: nowrap;
}
}

.actions {
Expand Down
4 changes: 2 additions & 2 deletions src/test-utils/dom/code-view/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import styles from "../../../code-view/styles.selectors.js";
export default class CodeViewWrapper extends ComponentWrapper {
static rootSelector: string = styles.root;

findContent(): ElementWrapper {
return this.findByClassName(styles.code)!;
findContent() {
Copy link
Member

Choose a reason for hiding this comment

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

changing the signature of this is not a backwards compatible change

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated this to be backwards compatible.

return this.findAllByClassName(styles["code-line"])!;
}

findActions(): ElementWrapper | null {
Expand Down
Loading