fix: Copy as K8s #2517
Merged
fix: Copy as K8s #2517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: John Cowen <john.cowen@konghq.com>
✅ Deploy Preview for kuma-gui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
kleinfreund
approved these changes
Apr 30, 2024
Signed-off-by: John Cowen <john.cowen@konghq.com>
|
I added the most minimal of comments here so we had something, I want to merge all our 3 CodeBlock components into one at some point anyway, so I guess I'll be back here at some point soonish, I can make things clearer then. |
|
This wasn't the fix in the end see #2513 (comment) Actual fix #2531 |
johncowen
added a commit
that referenced
this pull request
May 21, 2024
We have some places where we need eXtra eXtensions on top of KCopy This adds a thin wrapper over KCopy and builds things out so we can use XCopyButton (née KCopy) everywhere without it being problematic. I've kept `TextWithCopyButton` at least temporarily but it now just wraps over `XCopyButton`. We use `TextWithCopyButton ` quite a lot so I haven't gone through and search/replaced all the usages, I may do that here and then also `TextWithCopyButton` so we only have one copy button thing. I also added back in our clipboard debugging tooling that I originally added in #1981 and was removed in #2374 Last, I originally called this `XCopy` (like `KCopy`), but then realized this problem was probably caused by this naming (no explicit `*Button`) so I called it `XCopyButton` to make it clear this is a button and doesn't need to be/shouldn't be wrapped in another button Closes #2513 Related: #2517 (<== I thought this was a fix but it wasn't) Signed-off-by: John Cowen <john.cowen@konghq.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #2374 (comment)
Looks like this is one of those cases where we need to make sure we await for a promise and return its result rather than just return the promise to be awaited on elsewhere.
Probably why the original code was like:
kuma-gui/src/app/common/code-block/ResourceCodeBlock.vue
Lines 101 to 107 in 6c158ba
But we've lost that detail through lack some good comments and subsequent upgrades/refactors there. I'll probably either refactor this code slightly to make this clearer or add some clearer comments. Putting the PR up whilst I think about that a little.
As little sidenote: I really wanted to progress the dev time clipboard coping debug I added here #1981 into something we could also use for testing clipboard interactions.
Unfortunately we removed the clipboard debugging here #2374, but this PR shows why it would be good to get that back in and then also progressed into something we can use for e2e tests.
Closes #2513