fix(EuiCode): fix code block copy button not including the last character #6794

tkajtoch · 2023-05-22T20:09:39Z

Summary

This PR fixes #6585 that was caused by an incorrect regex being used in src/components/code/code_block_copy.tsx. Rest of the replace logic stays the same.

QA

General checklist

~~Checked in both light and dark modes~~
Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs (using @default if default values are missing) and playground toggles
Added documentation
~~Checked Code Sandbox works for any docs examples~~
Added or updated jest and cypress tests~
~~Checked for breaking changes and labeled appropriately~~
~~Checked for accessibility including keyboard-only and screenreader modes~~
~~Updated the Figma library counterpart~~
A changelog entry exists and is marked appropriately

src/components/code/code_block_copy.tsx

kibanamachine · 2023-05-22T20:40:52Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6794/

1Copenut

Current implementation works great in Safari, Firefox, Chrome, Edge.

All browsers except Firefox dropped the cursor at the end of the copied block. Firefox dropped the cursor on a newline, but that might be a browser detail? LMK if you want a second run through if/when you remove the second .replace() in code_block_copy.tsx.

tkajtoch · 2023-05-23T11:38:35Z

@1Copenut I just checked the copy behavior on Firefox and I couldn't replicate what you found. Do you remember what code block generated that new line when copying on Firefox?

kibanamachine · 2023-05-23T11:50:58Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6794/

1Copenut · 2023-05-23T13:38:09Z

@1Copenut I just checked the copy behavior on Firefox and I couldn't replicate what you found. Do you remember what code block generated that new line when copying on Firefox?

@tkajtoch As best I can recall, all of the blocks added that newline in MacOS Monterey, running latest Firefox. It was a consistent behavior for the popover, virtualized, multiline, and one-line copy examples.

cee-chen · 2023-05-23T15:52:50Z

Actually, I can repro, sorry:

export default () => (

  <div>In this example, the whiteSpace property is set to pre. All the whitespaces will be kept as is and the text only wraps when line breaks are in the content.</div>

);

That came from the whitespace example on https://eui.elastic.co/pr_6794/#/editors-syntax/code 😬

The HTML and JSON copyable examples work just fine however 💀

EDIT: This is also happening on Edge so I think this seems to specifically affect the whitespace behavior, and might have been why the \n\n regex was in place.

kibanamachine · 2023-06-01T14:52:22Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6794/

tkajtoch · 2023-06-01T15:42:12Z

@cee-chen @1Copenut I reverted the \n\n replacement removal and added a comment above why we need it so hopefully no one else makes the same mistake again

cee-chen · 2023-06-01T16:04:26Z

Thanks Tomasz - I can confirm the whitespace demo now copies with the correct newline spacing. The comment added is also perfect, thank you!

I'd like to see tests of some kind (looks like you could try RTL for clipboard assertions, if not I'd use Cypress) that assert the following:

A basic 'it copies the content' test, which asserts the pasted output is the same as the EuiCodeBlock children, which should be sufficient to check that the double newline issue doesn't happen
A regression test for this specific fix (i.e. children with a ? at the end of code lines)

cee-chen · 2023-06-01T19:13:41Z

Update to the above comment - I ended up adding the base for Cypress copy tests in #6824 while fixing another copy bug. I also went a little overboard and added a TODO regression test in that PR for this PR 😅

Once that PR lands, you should be able to go ahead and merge it in and remove/uncomment the last assertion to confirm that this now behaves as expected.

tkajtoch · 2023-06-02T13:09:12Z

@cee-chen gotcha, I'll rebase this branch and uncomment your test case when #6824 is merged 👍🏻

…cter if it's a question mark

…a single one when using the Copy button This change was first introduced in 7aeb30f ([EuiCodeBlock] Replace highlight.js with prism.js via refractor (elastic#4638)) but doesn't seem to be a business or product decision. We decided to remove it to make the copied content match what's displayed on screen.

…ines to a single one when using the Copy button" This reverts commit 8a90215c81432514493b1bb29ecd314d6792a05d.

upcoming_changelogs/6794.md

cee-chen

🚢

kibanamachine · 2023-06-02T17:44:37Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6794/

This is a backport EUI upgrade to Kibana v8.8.1 containing an `EuiCodeBlock` bugfix requested by the Observability team: #158644 (comment) ## [`77.1.5`](https://github.com/elastic/eui/tree/v77.1.5) **Bug fixes** - Fixed `EuiCodeBlock` potentially incorrectly ignoring lines ending with a question mark when using the Copy button. ([#6794](elastic/eui#6794)) - Fixed `EuiCodeBlock` to not include line numbers when copying content ([#6824](elastic/eui#6824)) - Fixed the expanded row animation on `EuiBasicTable` causing cross-browser Safari issues ([#6826](elastic/eui#6826)) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

@kqualters-elastic

## Summary `eui@81.2.0` ⏩ `eui@81.3.0` ✨ Highlights: - fixes #158644 - Adds a new Timeline icon for use within `EuiMarkdownEditor` (cc @kqualters-elastic) - Expandable rows used within `EuiBasicTable` and `EuiInMemoryTable` now have a faster and snappier expand animation ___ ## [`81.3.0`](https://github.com/elastic/eui/tree/v81.3.0) - Added `timelineWithArrow` glyph to `EuiIcon` ([#6822](elastic/eui#6822)) **Bug fixes** - Fixed `EuiCodeBlock` potentially incorrectly ignoring lines ending with a question mark when using the Copy button. ([#6794](elastic/eui#6794)) - Fixed `EuiCodeBlock` to not include line numbers when copying content ([#6824](elastic/eui#6824)) - Fixed the expanded row animation on `EuiBasicTable` causing cross-browser Safari issues ([#6826](elastic/eui#6826))

tkajtoch self-assigned this May 22, 2023

tkajtoch changed the title ~~fix(EuiCopy): fix code block copy button not including the last character~~ fix(EuiCode): fix code block copy button not including the last character May 22, 2023

tkajtoch force-pushed the fix/6585-code-block-copy-regex branch from 805ec38 to 07d8cdf Compare May 22, 2023 20:16

tkajtoch marked this pull request as ready for review May 22, 2023 20:16

tkajtoch requested a review from a team May 22, 2023 20:16

tkajtoch commented May 22, 2023

View reviewed changes

src/components/code/code_block_copy.tsx Show resolved Hide resolved

1Copenut reviewed May 22, 2023

View reviewed changes

tkajtoch force-pushed the fix/6585-code-block-copy-regex branch from aac77e3 to a1e0d93 Compare May 23, 2023 11:26

tkajtoch force-pushed the fix/6585-code-block-copy-regex branch from a1e0d93 to de36c35 Compare June 1, 2023 14:22

tkajtoch requested a review from cee-chen June 1, 2023 15:48

tkajtoch added 5 commits June 2, 2023 19:12

fix(EuiCode): fix code block copy button not including the last chara…

3a03d64

…cter if it's a question mark

chore: add changelog file

4ee0883

Revert "fix(EuiCode): Remove reducing two or more consecutive empty l…

171d786

…ines to a single one when using the Copy button" This reverts commit 8a90215c81432514493b1bb29ecd314d6792a05d.

docs: add clarification comment why \n\n replace is needed

91823c3

tkajtoch force-pushed the fix/6585-code-block-copy-regex branch from de36c35 to 91823c3 Compare June 2, 2023 17:12

cee-chen added 2 commits June 2, 2023 10:17

Merge branch 'main' into fix/6585-code-block-copy-regex

8632a20

Update regression test

4b9ae86

cee-chen reviewed Jun 2, 2023

View reviewed changes

upcoming_changelogs/6794.md Outdated Show resolved Hide resolved

changelog

6b0822b

cee-chen approved these changes Jun 2, 2023

View reviewed changes

tkajtoch enabled auto-merge (squash) June 2, 2023 17:33

tkajtoch merged commit b1ec18c into elastic:main Jun 2, 2023

cee-chen mentioned this pull request Jun 2, 2023

[8.8] Upgrade EUI to v77.1.5 elastic/kibana#158976

Merged

cee-chen mentioned this pull request Jun 5, 2023

Upgrade EUI to v81.3.0 elastic/kibana#159064

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(EuiCode): fix code block copy button not including the last character #6794

fix(EuiCode): fix code block copy button not including the last character #6794

tkajtoch commented May 22, 2023 •

edited by cee-chen

Loading

kibanamachine commented May 22, 2023

1Copenut left a comment

tkajtoch commented May 23, 2023

kibanamachine commented May 23, 2023

1Copenut commented May 23, 2023

cee-chen commented May 23, 2023

kibanamachine commented Jun 1, 2023

tkajtoch commented Jun 1, 2023

cee-chen commented Jun 1, 2023 •

edited

Loading

cee-chen commented Jun 1, 2023 •

edited

Loading

tkajtoch commented Jun 2, 2023

cee-chen left a comment

kibanamachine commented Jun 2, 2023

fix(EuiCode): fix code block copy button not including the last character #6794

fix(EuiCode): fix code block copy button not including the last character #6794

Conversation

tkajtoch commented May 22, 2023 • edited by cee-chen Loading

Summary

QA

General checklist

kibanamachine commented May 22, 2023

1Copenut left a comment

Choose a reason for hiding this comment

tkajtoch commented May 23, 2023

kibanamachine commented May 23, 2023

1Copenut commented May 23, 2023

cee-chen commented May 23, 2023

kibanamachine commented Jun 1, 2023

tkajtoch commented Jun 1, 2023

cee-chen commented Jun 1, 2023 • edited Loading

cee-chen commented Jun 1, 2023 • edited Loading

tkajtoch commented Jun 2, 2023

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 2, 2023

tkajtoch commented May 22, 2023 •

edited by cee-chen

Loading

cee-chen commented Jun 1, 2023 •

edited

Loading

cee-chen commented Jun 1, 2023 •

edited

Loading