nkbt / react-copy-to-clipboard

Copy-to-clipboard React component
MIT License
2.34k stars 125 forks source link

Always given "copy to clipboard" prompt #20

Closed thearrow closed 8 years ago

thearrow commented 8 years ago

Latest version of react-copy-to-clipboard (4.1.0)

OSX Chrome Version 49.0.2623.112 (64-bit) and OSX Chrome Version 52.0.2705.0 canary (64-bit)

Used inside a React 15 component, imported with import CopyToClipboard from 'react-copy-to-clipboard'

Used like so:

<CopyToClipboard text={window.location.href}
        onCopy={() => this.setState({ copied: true })}>
            <div className='photo-page-permalink'>
              {window.location.href}
            </div>
</CopyToClipboard>

Always gives the "Copy to clipboard: Ctrl+C, Enter" prompt, never copies silently.

Example page (http://nkbt.github.io/react-copy-to-clipboard/example/) in same browsers copies to clipboard silently.

On closing the prompt, an error is always thrown:

Uncaught TypeError: reselectPrevious is not a function

Any ideas?

Thanks!

nkbt commented 8 years ago

Hi, @thearrow

Always gives the "Copy to clipboard: Ctrl+C, Enter" prompt, never copies silently.

What do you mean by this? I do not quite get "gives the ... prompt" part.

thearrow commented 8 years ago

Instead of just copying to the clipboard silently, the component always displays the fallback browser prompt:

screen shot 2016-04-12 at 4 21 58 pm

Might be an issue with the underlying copy-to-clipboard package...

nkbt commented 8 years ago

That happens as a fallback when browser does not support execCommand natively. Which is quite strange since I am using Chrome Canary as well and it works totally fine for me.

nkbt commented 8 years ago

May it be possible due to some browser's privacy settings or ads blocker?

thearrow commented 8 years ago

I tried in both browsers in Incognito Mode (no extensions active) - same result.

Tried disabling ad blockers in normal mode - same result.

Browsers are at basically stock privacy settings - no changes to JS execution.

lucky-newbie commented 8 years ago

hi, do you have solved this problem? i met the same question. thanks

nkbt commented 8 years ago

I am still not able to reproduce this issue =(

lucky-newbie commented 8 years ago

en ... do you konw any other plugin to copy text in react ? i need to copy text in react, can you give me any suggestion? thank you very much!

mzamoras commented 8 years ago

I was having the same issue. This library uses "copy-to-clipboard" which happens to use "toggle-selection" library. At the very top of "toggle selection"(index.js) this line is causing troubles:

var module = module || {};

And after removing that line everything worked just perfectly. Without removing it, the "copy-to-clipboard" library, is unable to reach properly this module using:

var deselectCurrent = require("toggle-selection");  // returns {} instead of a function

Just check your console for an error with "reselectPrevious is not a function", because that error somehow ends up displaying that prompt which should only be displayed as fallback for unsupported browsers.

Not sure who should fix this issue or if is related directly to this library or author, but at least it's a temporary fix.

nkbt commented 8 years ago

Hm that looks like some issue with build process. Do you mind sharing your build config, @mzamoras?

mzamoras commented 8 years ago

Sure, no problem ... but ... what you mean by build config? hope this helps...
OSX 10.11.4, fixed and tried on Safari 9.1 and Chrome 49.0.2623.112

my package.json:

 {
"devDependencies": {
    "babel-plugin-transform-runtime": "^6.7.5",
    "babel-preset-react-hmre": "^1.1.1",
    "babelify": "^7.2.0",
    "chai": "^3.5.0",
    "envify": "^3.4.0",
    "enzyme": "^2.3.0",
    "expect": "^1.20.1",
    "expect-jsx": "^2.5.1",
    "gulp": "^3.9.1",
    "mocha": "^2.4.5",
    "react-addons-test-utils": "^15.0.2",
    "redux-devtools": "^3.2.0",
    "redux-immutable-state-invariant": "^1.2.3"
  },
"dependencies": {
    "antd": "^0.12.14",
    "bourbon": "^4.2.7",
    "browserify-hmr": "^0.3.1",
    "classnames": "^2.2.3",
    "immutable": "^3.8.1",
    "laravel-elixir": "^5.0.0",
    "laravel-elixir-browserify": "^0.8.1",
    "lodash.merge": "^4.3.5",
    "material-ui": "^0.14.4",
    "normalizr": "^2.0.1",
    "object.assign": "^4.0.3",
    "pusher": "^1.2.1",
    "pusher-js": "^3.1.0",
    "react": "^15.0.1",
    "react-addons-css-transition-group": "^15.0.2",
    "react-copy-to-clipboard": "^4.1.0",
    "react-dom": "^15.0.1",
    "react-mousetrap": "0.0.8",
    "react-redux": "^4.4.5",
    "react-router": "^2.2.4",
    "react-router-redux": "^4.0.2",
    "redux": "^3.4.0",
    "redux-thunk": "^2.0.1",
    "reselect": "^2.5.1",
    "semantic-ui": "^2.1.8"
  }
}
nkbt commented 8 years ago

@mzamoras well, I meant like .babelrc, webpack.config or as I can see you use browserify and gulp (which probably might be the case). I am not a browserify user, but I guess it can treat module somewhat differently from how webpack does it.

mzamoras commented 8 years ago

.babelrc: //environment name intentionally changed from development to devol, to be able to make some tests with mocha and use hot module reload for development

{
  "presets": ["es2015", "react"],
  "plugins": ["transform-runtime"],
  "env": {
    "devol": {
      "presets": ["react-hmre"]
    }
  }
}

gulpfile: using laravel elixir


var elixir = require( 'laravel-elixir' );
var elixirConfig = elixir.config;
var bConfig = elixirConfig.js.browserify;

if ( elixirConfig.production == true ) {
    process.env.NODE_ENV = 'production';
}

bConfig.plugins.push({
    name: 'browserify-hmr',
    options:{}
});

/*
 |--------------------------------------------------------------------------
 | Elixir Asset Management
 |--------------------------------------------------------------------------
 |
 | Elixir provides a clean, fluent API for defining some basic Gulp tasks
 | for your Laravel application. By default, we are compiling the Sass
 | file for our application, as well as publishing vendor resources.
 |
 */

elixir( function (mix) {

    mix.sass( 'app.scss' );
    mix.scripts( [ 'base.js' ], 'public/js/base.js' );
    mix.styles( [ '../../../node_modules/antd/lib/index.css','../semantic/dist/semantic.css' ], 'public/css/antd.css' );
    mix.browserify( 'react/app.js', 'public/js/bundle.js');

    if (! elixir.config.production) {
        process.env.NODE_ENV = 'devol';
    }

    mix.browserSync( {

        proxy: {
            target: "http://test.com",  // locally resolved domain for development
            ws    : true
        },

        open: false,
        reloadOnRestart: true,
        browser: "google chrome",
        injectChanges: true,

        baseDir   : 'public',

        files: [
            'public/css/**/*.css',
            //'public/**/*.html',
            elixirConfig.appPath + '/**/*.php',
            elixirConfig.viewPath + '/**/*.php'
        ]
    } );
} );

´´´
nkbt commented 8 years ago

ouch... I thought I'd be able to replicate it, but probably not =(.

Hi, @sudodoki! Do you have any idea?

mzamoras commented 8 years ago

did you try to update the "toggle-selection" library to the latest? ... in your system the header of that file is exactly to the one I did comment? If so, do you have the same results printing deselectCurrent variable?

var module = module || {};

"copy-to-clipboard" library use it with :

var deselectCurrent = require("toggle-selection");

/* Inside this library I tried to check the value of deselectCurrent */
console.log( deselectCurrent );  //  Returns empty object, {}
/* This is causing the incorrect catch on this library's try-catch */ 
stevenmusumeche commented 8 years ago

I'm also having this issue with 4.1.0

sudodoki commented 8 years ago

Well, the original issue might be caused by 1) you are trying to call 'copy-to-clipboard' when there's no interaction by user (clicks/keydowns, etc).

Copy commands triggered from document.execCommand() will only affect the contents of the real clipboard if the event is dispatched from an event that is trusted and triggered by the user, or if the implementation is configured to allow this

2) there's an error thrown somewhere that leads to IE fallback prompt I think after taking a look at this I will be adding some more info to logging when debug option is set to true and let you know when you can test the build. That way you would know whether that is a SecurityError or something else,

sudodoki commented 8 years ago

I updated copy-to-clipboard in master branch, could you try checking with it and seeing what the error would be?

stevenmusumeche commented 8 years ago

Hi @sudodoki and thank you for the reply. We ended up using something custom instead of this package. Directly calling execCommand works fine so I believe there is some issue in the library at play here.

nkbt commented 8 years ago

I reckon this one needs to update dep too :). Will do as soon as I am back from holidays (or find better internet here)

nkbt commented 8 years ago

Is https://github.com/nkbt/react-copy-to-clipboard/issues/24 the same the problem as here?

ffigiel commented 8 years ago

I had the same problem, but installing newer copy-to-clipboard (3.0.2 specifically) fixed the problem.

nkbt commented 8 years ago

Published + react-copy-to-clipboard@4.1.1

Let me know if issue is still there and reopen ticket!

Thanks!

ffigiel commented 8 years ago

Installed 4.2.0 and it works, thanks!

nkbt commented 8 years ago

Awesome! Thanks for report On Tue, 5 Jul 2016 at 21:38, Filip notifications@github.com wrote:

Installed 4.2.0 and it works, thanks!

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/nkbt/react-copy-to-clipboard/issues/20#issuecomment-230455169, or mute the thread https://github.com/notifications/unsubscribe/AAKsoAdiTFq4V0eAINA1HIq6L7sJE5qjks5qSkINgaJpZM4IF4ES .

mzamoras commented 8 years ago

I did update to the 4.2.1 and the problem still exist, then tried the 4.2.0 , still the same.

Then I re-installed version 4.2.1 and modified the package ( 'toggle-selection' ), and every thing starts working again.

You should really check that package, it's not just preventing the direct copy or showing the alert box, is also throwing a warning on the console. I mean, your library works awesome! And I think those errors are not related to your work but definitely is something that might be causing some troubles or preventing the usage of this library...

Also I've been thinking that it could be related to the var-word "module" and how browserify or babel handle it or maybe is webpack the one who is fixing it for you, because that single line is trying to define "module", although is not registered as an actual keyword, but when debugging the value assigned to that var was always an empty object {}, so no calls or properties are available. In fact, deleting it and the fact that the library works means that it might be not necessary.

You should try deleting it and see if it still works for you as expected.

Finally, after writing this lines, I did put the deleted line back and re-compile everything using webpack ... then ... the error was gone! then remove the line again and tested it again with webpack and also working ...

I like browserify better, so I prefer removing the line...

nkbt commented 8 years ago

@mzamoras have you tried opening an issue in toggle-selection repo? I mean I really don't have any sort of control over it... Most likely its build is not perfect and has to be fixed.

nkbt commented 8 years ago

https://npmcdn.com/toggle-selection@1.0.4 -- yeah this one is the only one that has that weird

var module = module || {};

I've never seen anyone doing this. Pretty strange practice. Worth opening an issue

sudodoki commented 8 years ago

@nkbt @mzamoras do you have suggestion on what should be there would gladly modify that piece?

mzamoras commented 8 years ago

@sudodoki sure, just delete that line.

Look https://nodejs.org/api/modules.html#modules_accessing_the_main_module , here Node explains the module or module exports object and usage.

As far as I've read, there is a "module pattern", see example https://darrenderidder.github.io/talks/ModulePatterns/#/, but that specific line is rare and is not included anywhere in the pattern definition, not just in this page, most of the explanations of the pattern definition don't include it.

So you would normally use one of this

//file starts
var toggle = function(){
    //... your code here
}
module.exports = toggle; 
//file ends

or this form

//file starts
module.exports = function(){
    //your code here
}
//file ends

there is no real need to protect against undefined word "module".

mzamoras commented 8 years ago

@nkbt I just did fork the repo and made a pull a request, so waiting to be accepted by @sudodoki , but the package needs to be also updated on npmjs.com

nkbt commented 8 years ago

👌 On Tue, 26 Jul 2016 at 04:14, mzamoras notifications@github.com wrote:

@nkbt https://github.com/nkbt I just did fork the repo and made a pull a request, so waiting to be accepted by @sudodoki https://github.com/sudodoki , but the package needs to be also updated on npmjs.com

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nkbt/react-copy-to-clipboard/issues/20#issuecomment-235036629, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKsoCw40ZsSMSscm7Mb0AKoFQ6W9ny2ks5qZP0GgaJpZM4IF4ES .

sudodoki commented 8 years ago

@mzamoras published https://www.npmjs.com/package/toggle-selection @ 1.0.5, so it should be fixed now, can you be so kind to check?

nkbt commented 8 years ago

I reckon you need to publish copy to clipboard too. And I need to publish this one after that On Wed, 27 Jul 2016 at 06:47, Джон, просто Джон notifications@github.com wrote:

@mzamoras https://github.com/mzamoras published https://www.npmjs.com/package/toggle-selection @ 1.0.5, so it should be fixed now, can you be so kind to check?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nkbt/react-copy-to-clipboard/issues/20#issuecomment-235399753, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKsoGXO4fLXmt8jJNF7nW2EZc0l09mKks5qZnH9gaJpZM4IF4ES .

sudodoki commented 8 years ago

@nkbt copy-to-clipboard depends on ^1.0.3 and I published 1.0.5 so it should work

nkbt commented 8 years ago

Just in case I republished lib so prebuilt script has correct versions of deps.

mzamoras commented 8 years ago

sorry to answer that late, but the problem is fixed now, using react-copy-to-clipboard@4.2.2 with toggle-selection@1.0.5 !! @nkbt @sudodoki thank you both!!

nkbt commented 8 years ago

\o/ Awesome

valentinvoilean commented 7 years ago

I still get this error when I try to test it with Jest & Enzyme. I have installed toggle-selection 1.0.5 & react-copy-to-clipboard 4.2.3

This is my code:

_handleCopy() {
        this.setState({copied: true});
    }

    render() {
        const buttonClass = `${styles.copy} ${this.state.copied ? styles.copied : ''}`;

        return (
            <div className={styles.codeContainer}>
                <code className={styles.code}>{this.props.text}</code>
                <CopyToClipboard text={this.props.text} onCopy={this._handleCopy}>
                    <button className={buttonClass}>Click to copy</button>
                </CopyToClipboard>
            </div>
        );
    }

And this is the code used for testing:

it('should update the button class on click', () => {
        wrapper.find('button').simulate('click');
        expect(wrapper.state().copied).toBe(true);
    });
valentinvoilean commented 7 years ago

Most probably this problem is because Jest it's using Node to run all the tests unlike the Karma which can be configured to use Chrome launcher.

As a workaround, I did this:

it('should update the button class on click', () => {
        wrapper.instance()._handleCopy();
        expect(wrapper.state().copied).toBe(true);
    });

But doing this, I'm not testing anymore the CopyToClipboard component.

nkbt commented 7 years ago

Well, you should not test 3rd party components. Mock it out completely, replace with the component that will run you callback and you will now test your component instead!

Cheers On Wed., 7 Dec. 2016 at 22:27, Valentin-Marian Voilean < notifications@github.com> wrote:

Most probably this problem is because Jest it's using Node to run all the tests unlike the Karma which can be configured to use Chrome launcher.

As a workaround, I did this:

it('should update the button class on click', () => { wrapper.instance()._handleCopy(); expect(wrapper.state().copied).toBe(true); });

But doing this, I'm not testing anymore the CopyToClipboard component.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nkbt/react-copy-to-clipboard/issues/20#issuecomment-265423690, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKsoL8xn0q6JmMHQX9nx_ru4PyYj4qZks5rFpgAgaJpZM4IF4ES .

mhuggins commented 6 years ago

For anyone having trouble simulating in tests (Jest specifically), here's what I did to get it working.

Assuming your package.json has a section like the following:

"build": {
  "setupFiles": [
    "./__tests__/setupTests.js",
  ]
}

You can add the following to your test setup file (mine being setupTests.js):

// Polyfill window prompts to always confirm.  Needed for react-copy-to-clipboard to work.
global.prompt = () => true;

// Polyfill text selection functionality.  Needed for react-copy-to-clipboard to work.
// Can remove this once https://github.com/jsdom/jsdom/issues/317 is implemented.
const getSelection = () => ({
  rangeCount: 0,
  addRange: () => {},
  getRangeAt: () => {},
  removeAllRanges: () => {}
});

window.getSelection = getSelection;
document.getSelection = getSelection;
Desintegrator commented 5 years ago

faced this issue on 5.0.1