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

"partial" smartCopy behavior is surprising #13

Open
benwiley4000 opened this issue Mar 3, 2020 · 1 comment
Open

"partial" smartCopy behavior is surprising #13

benwiley4000 opened this issue Mar 3, 2020 · 1 comment

Comments

@benwiley4000
Copy link

benwiley4000 commented Mar 3, 2020

Not really a bug but it seems like the "all" and "partial" options do the same thing for smartCopy. I looked through the source code and couldn't find any differences in behavior. I assume partial was intended to create an expanded selection if the selection text is a substring that contains the ellipsis? If you don't plan to implement that feature maybe it would be less confusing for that option to be deprecated (but still work until a major release).
I re-read the code and realized that the trigger for each option is different (i.e. partial works even if the selection is an inexact match), but in each case the copied data becomes the full text input, which seems wrong for substring selections.

@benwiley4000
Copy link
Author

benwiley4000 commented Mar 3, 2020

Sorry I reread the code and realized that partial actually is different, although I think its behavior may be incorrect since it doesn't actually select the substring the user chose.

I think it could be done more precisely like this:

  1. Create another piece of state called excludedSubstring which is saved at the same time as truncatedText and contains exactly what it sounds like
  2. When smartCopy is "partial" and a copy is made, you can produce the expanded substring with:
      if (smartCopy === 'partial') {
        event.preventDefault();
        const clipboardData = event.clipboardData || window.clipboardData || event.originalEvent.clipboardData;
        clipboardData.setData('text/plain', selectedText.replace(
          this.props.ellipsis,
          this.state.excludedSubstring
        ));
      }

@benwiley4000 benwiley4000 changed the title "all" and "partial" smartCopy are the same? "partial" smartCopy behavior is surprising Mar 3, 2020
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

No branches or pull requests

1 participant