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

[DYN-6361] Bugfix copy/paste operation conflict between library search and Workspace #14998

Conversation

Enzo707
Copy link
Contributor

@Enzo707 Enzo707 commented Mar 4, 2024

Purpose

Fix Library search copy/paste operation misbehavior that affects Workspace.
Ref.: DYN-6361

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Reviewers

@QilongTang

FYIs

@avidit

Copy link

github-actions bot commented Mar 4, 2024

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

@@ -237,6 +237,8 @@ internal void RefreshLibraryView(WebView2 browser)
/// <param name="text">text to be added to clipboard</param>
internal void OnCopyToClipboard(string text)
{
dynamoViewModel.Model.ClipBoard.Clear();
Clipboard.Clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to clear both clipboard?

Copy link
Contributor Author

@Enzo707 Enzo707 Mar 5, 2024

Choose a reason for hiding this comment

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

I forgot to remove that statement, I've just update the PR.
However I'm going to set the PR as a draft as I realized there's an use case that need to be addressed before wrap this up. If you perform a node paste into workspace, then copy anything from outside Dynamo and paste it into library search it will paste the node as well, as it remains on clipboard.

Copy link
Contributor

@RobertGlobant20 RobertGlobant20 Mar 6, 2024

Choose a reason for hiding this comment

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

@Enzo707 this is the approach that I was talking about when implemeting the functionality of copying an external text to Dynamo, then you need to do the next steps to reproduce the bug:

  1. Add a Number node in workspace
  2. Select the Number node and press Ctrl + C
  3. Then press Ctrl + V (the node will be added/copied in workspace)
  4. The go to an external app (like notepad), select a text and press Ctrl + C
  5. Return to Dynamo, go to the Search TextBox in Library and press Ctrl + V
    The problem is that is copying the text from Notepad to the Search TextBox in Library and also adding/copying a Number node in workspace.

Then for fixing this problem my approach was to add a variable that holds the focused element, in this case when the SearchTextBox is focused or lost focus a value will be set in the variable.
I've already tested and seems that is working as expected, please double check.

This are the lines used for subcribing to the focus event and lost focus event(I executed it using the DeveloperTools), probably needs to be added in React when the page is already loaded because when I added it to library.html was crashing the webpage.

document.getElementsByClassName("SearchInputText")[0].addEventListener("focusin", (event) => {
window.chrome.webview.postMessage(JSON.stringify({ "func": "FocusUpdated", "data": "Focused" }));
});

document.getElementsByClassName("SearchInputText")[0].addEventListener("focusout", (event) => {
window.chrome.webview.postMessage(JSON.stringify({ "func": "FocusUpdated", "data": "LostFocus" }));
});

This is the branch in which I uploaded the code (the last commit has the updated code):
https://github.com/RobertGlobant20/Dynamo/tree/DYN-6361-CopyPaste-FixProposal

@QilongTang QilongTang added this to the 3.1 milestone Mar 5, 2024
@Enzo707 Enzo707 marked this pull request as draft March 5, 2024 16:06
@QilongTang QilongTang marked this pull request as ready for review March 8, 2024 03:26
@QilongTang
Copy link
Contributor

Merge for now and other edge cases handled by different tasks

@QilongTang QilongTang merged commit 0883984 into DynamoDS:master Mar 8, 2024
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants