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.

Sign up for GitHub

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

Jump to bottom

feat(code-block): added code block component #3937

Merged
merged 6 commits into from
Mar 24, 2021
Merged

feat(code-block): added code block component #3937

merged 6 commits into from
Mar 24, 2021

Conversation

mcoker
Copy link
Contributor

@mcoker mcoker commented Mar 19, 2021

fixes #3907

@mcoker mcoker requested a review from mcarrano March 19, 2021 01:03
@mcoker mcoker mentioned this pull request Mar 19, 2021
Closed
@patternfly-build
Copy link

patternfly-build commented Mar 19, 2021 •
edited
Loading

Preview: https://patternfly-pr-3937.surge.sh

A11y report: https://patternfly-pr-3937-coverage.surge.sh

CSS Size Report
NameCurrentPreviousDiff %
components/ClipboardCopy/clipboard-copy.css4.9 kB2.5 kB49.55
components/CodeBlock/code-block.css1.5 kBNaN kBNaN
components/ExpandableSection/expandable-section.css3.5 kB2.9 kB16.36
components/Form/form.css18.1 kB18.3 kB-1.43
patternfly-no-reset.css1.1 MB1.1 MB0.39
patternfly.css1.1 MB1.1 MB0.39
patternfly.min.css980.8 kB976.9 kB0.40

mcarrano
mcarrano reviewed Mar 19, 2021
View reviewed changes
Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

Looks good to me @mcoker . You @lwigh?

@mcarrano mcarrano mentioned this pull request Mar 19, 2021
Closed
mattnolting
mattnolting reviewed Mar 23, 2021
View reviewed changes
Copy link
Contributor

@mattnolting mattnolting left a comment

Choose a reason for hiding this comment

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

This looks awesome, great work! Just need to add white-space controls, and it's GTG!

Screen Shot 2021-03-23 at 2 09 17 PM

src/patternfly/components/CodeBlock/code-block-code.hbs Outdated
{{#if code-block-code--attribute}}
{{{code-block-code--attribute}}}
{{/if}}>
{{~>@partial-block~}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to add ~?

src/patternfly/components/CodeBlock/code-block.scss Outdated

.pf-c-code-block__header {
display: flex;
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think you need this.

Suggested change
width: 100%;

@mcoker mcoker changed the title feat(code-block): added code block variant feat(code-block): added code block component Mar 23, 2021
@mcoker mcoker requested a review from tlabaj March 24, 2021 17:16
mattnolting
mattnolting approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@mattnolting mattnolting left a comment

Choose a reason for hiding this comment

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

LPTM! 🎉

@lwrigh
Copy link

lwrigh commented Mar 24, 2021

The demo looks great to me!

@mcarrano
Copy link
Member

This looks good @mcoker . My only question is whether the gap between the bottom of the text and the expansion widget needs to be so wide. Is that just a function of the block contents or is that built-in spacing?

@mcoker
Copy link
Contributor Author

mcoker commented Mar 24, 2021

@mcarrano That's an artifact of whitespace management in our examples, @redallen is working on a fix for us. This is what the expand button will look like when we can better manage the whitespace.

Screen Shot 2021-03-24 at 2 28 16 PM

Screen Shot 2021-03-24 at 2 30 24 PM

mcarrano
mcarrano approved these changes Mar 24, 2021
View reviewed changes
Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification @mcoker . I approve then!

@mcoker mcoker merged commit 0c70317 into patternfly:master Mar 24, 2021
@redallen
Copy link
Contributor

🎉 This PR is included in version 4.94.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sahil143
Copy link
Member

sahil143 commented Mar 29, 2021 •
edited
Loading

There should not be a need to put <br /> tag on each line inside <pre/>. It should handle the formatting itself.

@christianvogt
Copy link

@mcoker I agree with @Sahil here. If we're displaying a code block, I don't want to have to replace new lines with a break tag.

@redallen
Copy link
Contributor

The reason that the <br> tags are there is a workspace bug. #3956 should fix it.

@mcoker mcoker mentioned this pull request Apr 14, 2021
Merged
@mcoker mcoker deleted the issue-3907 branch January 7, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattnolting mattnolting mattnolting approved these changes

@mcarrano mcarrano mcarrano approved these changes

@tlabaj tlabaj Awaiting requested review from tlabaj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce Code Block component
8 participants
@mcoker @patternfly-build @lwrigh @mcarrano @redallen @sahil143 @christianvogt @mattnolting