Uninitialises after focus on yoked text input

[leanne-tite-jg]

I've added react-fastClick to a React form-based app. We hd the same issue as in #22 and removed the plugin until this issue was fixed. Since adding it back we have noticed the following behaviour on iPhone iOS 9.3.1 Safari (but I'm not sure this issue is device-specific):

• We have a radio button group with radio buttons that when clicked (or their labels clicked) updates the value in a text field. When that text field is focused and the mobile keyboard is rendered, the fastClick stops working for all controls in the React component (not just the radio buttons but all other rendered clickable controls), as if react-fastClick has been been de-initialised. The text input and radio buttons share a change handler, but note that this behaviour is observed even if the text input field is not changed, just focusing on it causes the issue

I am initialising fastClick with:

import 'react-fastclick';

on index.js which contains a root component which renders the react router routes (on one of which the above behaviour is manifested) and does all the other set up. This root component is in turn rendered. So basically we are initialising react-fastClick at the very top of the application.

[leanne-tite-jg]

Note: Once this behaviour is triggered it is carried over to any other component that is newly mounted henceforth e.g. when you transition to a new route and mount a new component the fastClick is still absent

[JakeSidSmith]

I'm out of the country at the moment, and don't have access to a computer, but if you can give me a little more info I'll have a think about it before I get back (in a couple of days).

Apart from the lack of fastclick, does everything else function correctly? Click and change events firing etc?

Can you see if there are any errors? You'll have to use the Safari debug tools to check with an iPhone.

Can you check if this problem persists for other devices? I'd recommend using the Chrome dev tools in device mode.

Could you post an example of the mark-up for your labels and inputs (the checkbox and text input). So I can see how they are related. E.g. what are child elements/siblings and if they are using htmlFor, etc.

[leanne-tite-jg]

Hi Jake sorry for the delay. Here are the answers:

• Yes everything else works as expected
• I've tried the app on Android phones [Nexus 4 and Nexus 6] in latest Chrome [50.x] and in fact we don't seem to get fast click at all on those devices (going by what i see on the screen when i interact), even before the above bug would be triggered
• On the iOS device [iOS 9.3.1, using Safari] no errors are observed in the console following the behaviour that triggers the bug (debugging using the Safari console on a mac paired with the iPhone)
• I've attached 3 screen shots:
• The first is the UI itself. The text input prefixed with the '£' is what triggers the the bug when focused. The buttons above populate the text input when clicked but equally the text input can be manually adjusted. When a figure that matches a button is manually inputted, the corresponding button highlights
• The second is the React dev tools view. The radio button group is RadioGroupWithLabel component and the text input is TextInputWithLabel component
• The third is the rendered DOM view. The radio button group is .dna-radio-group component and the text input is .dna-input-select-group > .donation-amount-field

[uinz]

warning.js:44 Warning: This synthetic event is reused for performance reasons. If you're seeing this, you're adding a new property in the synthetic event object. The property is never released. See https://fb.me/react-event-pooling for more information.

  "dependencies": {
    "amazeui-touch": "^1.0.0-beta.1",
    "babel-eslint": "^6.0.4",
    "babel-loader": "^6.2.4",
    "babel-preset-es2015": "^6.6.0",
    "babel-preset-react": "^6.5.0",
    "babel-preset-stage-0": "^6.5.0",
    "babel-regenerator-runtime": "^6.5.0",
    "classnames": "^2.2.5",
    "css-loader": "^0.23.1",
    "file-loader": "^0.8.5",
    "html-webpack-plugin": "^2.16.1",
    "react": "^15.0.2",
    "react-dom": "^15.0.2",
    "react-fastclick": "^2.1.1",
    "react-redux": "^4.4.5",
    "react-router": "^2.4.0",
    "react-router-redux": "^4.0.4",
    "redux": "^3.5.2",
    "redux-logger": "^2.6.1",
    "redux-promise": "^0.5.3",
    "redux-thunk": "^2.1.0",
    "style-loader": "^0.13.1",
    "stylus": "^0.54.5",
    "stylus-loader": "^2.0.0",
    "url-loader": "^0.5.7",
    "webpack": "^1.13.0"
  },
  "devDependencies": {
    "webpack-dev-server": "^1.14.1",
    "eslint": "^2.9.0",
    "eslint-config-airbnb": "^9.0.1",
    "eslint-plugin-babel": "^3.2.0",
    "eslint-plugin-import": "^1.8.0",
    "eslint-plugin-jsx-a11y": "^1.2.0",
    "eslint-plugin-markdown": "^1.0.0-beta.2",
    "eslint-plugin-react": "^5.1.1"
  },

$ node -v
v5.7.1
$ npm -v
3.6.0

[JakeSidSmith]

When you say you don't get fastclicks on Android devices, do you mean that the event.fastclick property is not defined? As there should be no delay on Android devices anyway (as far as I know).

Seems that is could be due to React preventing setting event properties in 0.15.

I'll test this out myself tonight and see if I can fix it. Seems that event.persist() could do the trick.

Thanks for all your help. :)

[leanne1]

Hi Jake, no worries - please note I'm using react 0.14. The package.json posted above is not mine :) When i say no fast clicks on Android I'm basing purely on what i see when I click i.e. there is a delay on the click, its not based on evidence as such

[JakeSidSmith]

Oh, woops. Was on my mobile, hard to see everything on such a small screen. XD

Thanks you all for your help in that case.

I'll have a look at both 0.14 and 0.15. Seems like 0.15 at least should be a reasonably easy fix. :)

P.s. 2 GitHub accounts?

[leanne1]

Yes 2 Github accounts!

On 16 May 2016 at 22:04, Jake notifications@github.com wrote:

Oh, woops. Was on my mobile, hard to see everything on such a small
screen. XD

Thanks you all for your help in that case.

I'll have a look at both 0.14 and 0.15. Seems like 0.15 at least should be
a reasonably easy fix. :)

P.s. 2 GitHub accounts?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#27 (comment)

[JakeSidSmith]

@yinzSE Would you mind opening another issue with the information you've posted above. Appears to be related specifically to React 0.15, and therefore I think the 2 are unrelated (although similar).

[JakeSidSmith]

@leanne-tite-jg / @leanne1 I can't seem to reproduce the problem you're having with React 0.14.

Could you add some onClick handlers (I'd suggest keeping these in a separate branch as I may need to get some more info following the results) to the input and buttons like the following:

handleClick: function (event) {
  console.log(event.type, event.fastclick);
}

<input onClick={this.handleClick} />

And let me know what the outcome is when clicking on a few of them?

1. Clicking the buttons before the input (since the input seems to be the cause of the problem)
2. Clicking the input
3. Clicking the buttons following clicking the input
4. Any other elements that you feel might be useful to look into

My current thought is that there may be a memory leak somehow in your app, or react-fastclick, or something else causing slow rendering. Should get a rough idea from the results of that handler. :)

[leanne-tite-jg]

Hi Jake I've done as the above and here are the results:

1. Radio input ('button') - note that clicks do not fire as the radio is hidden and the click is on the label. Clicking the label fires an onChange event on the radio. When i click the label this is the logging i get:

RADIO LABEL EVENT FASTCLICK [event.fastclick]: true 
RADIO LABEL EVENT TYPE: click 
RADIO INPUT EVENT FASTCLICK [event.fastclick]: undefined 
RADIO INPUT EVENT TYPE: change 

2. When i click the text input that triggers the bug i get:

TEXT INPUT EVENT FASTCLICK [event.fastclick]: true
TEXT INPUT EVENT TYPE: click

3. When i click the buttons again following clicking in the input i get (so basically no change, even though the fast click is visibly missing):

RADIO LABEL EVENT FASTCLICK [event.fastclick]: true
RADIO LABEL EVENT TYPE: click
RADIO INPUT EVENT FASTCLICK [event.fastclick]: undefined
RADIO INPUT EVENT TYPE: change

However, I have thought of what might be an issue here. The UI components on the page (radio group, text input with currency prefix) are imported from our own UI library. The UI library contains CSS and, for some elements, also JS enhancements. The JS enhancements are pure OOJS. So for example, the text input you see is enhanced with JS that handles changing the currency prefix. The JS enhancements on this inout are instantiated and handled within the React component as:

/**
     * Initialise input prefix
     */
    componentDidMount() {
        const { inputPrefix, id } = this.props;

        if (inputPrefix) {
            this.inputPrefix = new DNAInputPrefix({
                el: this.refs[id]
            });
        }
    }

/**
     * Update input prefix
     */
    componentDidUpdate(prevProps) {
        if (prevProps.inputPrefix !== this.props.inputPrefix) {
            this.inputPrefix.updatePrefix();
        }
    }
    /**
     * Destroy input prefix
     */
    componentWillUnmount() {
        if (this.inputPrefix) {
            this.inputPrefix = null;
        }
    }

So effectively this code short-circuits React. The DNAInputPrefix class looks like this:

import {emitter} from 'dna-core/components/eventemitter';

/**
 * Class representing a dna input-prefix
 * @classdesc Constructor for a dna input-prefix
 * @param {Object} options.el. Class instance's parent element
 */
export default class InputPrefix {
    constructor(options = {}) {
        this.el = options.el;

        // Cancel instantiation if DOM not as expected
        if (!this.verifyDOM()) {
            emitter.emit('dna.InputPrefix.didCancelInit', {
                emitter: this
            });
            return {};
        }

        this.init();

        // Everything inited as expected
        emitter.emit('dna.InputPrefix.didInit', {
            emitter: this
        });

        return this;
    }
    /**
     * Verify the required DOM nodes are present
     * @description Check that the object instance's parent element, and any necessary child nodes, are present in the DOM
     * @returns {Boolean} Whether all DOM nodes are present or not
     */
    verifyDOM() {
        let hasVerified = true;
        if (!this.el || !this.el.hasAttribute('data-dna-input-prefix')) {
            hasVerified = false;
        }
        return hasVerified;
    }
    /**
     * Initialise the class
     * @description
     */
    init() {
        this.inputPadding = parseInt(getComputedStyle(this.el).paddingLeft, 10);
        this.wrapInput();
        this.createTokenEl();
        this.appendToken();
        this.setPrefix();
    }
    /**
     * Create wrapper around input
     */
    wrapInput() {
        if (!this.el.parentNode.classList.contains('dna-input-prefix')) {
            this.wrapper = document.createElement('span');
            this.wrapper.classList.add('dna-input-prefix');
            this.el.parentNode.insertBefore(this.wrapper, this.el);
            this.wrapper.appendChild(this.el);
        }
    }
    /**
     * Create element to hold prefix
     */
    createTokenEl() {
        this.token = document.createElement('span');
        this.token.classList.add('dna-input-prefix-token');
    }
    /**
     * Append the prefix token to the input wrapper
     */
    appendToken() {
        this.wrapper.appendChild(this.token);
    }
    setPrefix() {
        this.setTokenLabel();
        this.getPrefixWidth();
        this.setInputPadding();
    }
    /**
     * Set the text (label) of the prefix token to the data-dna-input-prefix value
     */
    setTokenLabel() {
        this.token.innerHTML = this.el.dataset.dnaInputPrefix;
    }
    /**
     * Get the computed width of the label
     */
    getPrefixWidth() {
        return this.token.offsetWidth;
    }
    /**
     * Set padding on the input to account for prefix width plus its own padding
     */
    setInputPadding() {
        this.el.style.paddingLeft = `${this.inputPadding + this.getPrefixWidth()}px`;
    }
    /**
     * Reset the prefix label to latest data-dna-input-prefix value
     */
    updatePrefix() {
        this.setTokenLabel();
        this.setInputPadding();
    }
}

However, notice that none of methods get invoked just from focusing the input, and indeed a log in this.inputPrefix.updatePrefix method confirms it does not get called unless the currency is changed (the currency select list is changed). Note also that just instantiating the DNA class does not seem to eliminate the fastclick, and the behaviour does not seem to worsen over time (e,g, the responsiveness of the click does not seem to get worse over time once it is 'removed').

React do say that you can use third-party library code in a react component by directly accessing a DOM Node with refs, as we are doing, [http://facebook.github.io/react/docs/working-with-the-browser.html#refs-and-finddomnode]. We tried this initially experimentally and have so far not noticed any issues with doing so. But i wonder if this is linked to the problem we see?

[JakeSidSmith]

Hard to say from a quick look, if this could be what's causing the delay... but I would say however, that although React states that you can access DOM nodes using refs, you should not mutate the DOM. I've had problems with similar things before. I'm positive that what you're trying to achieve could be done in a much more React friendly way.

One possible fix could be to return false from the shouldComponentUpdate method, which will prevent React from also trying to modify the DOM, though I can't be certain - I've also had problems with that, it will likely stop other things you want React to update from working correctly.

A few questions:
Does InputPrefix not have a render method?
Is your InputPrefix class the same as the DNAInputPrefix used above?
Why did you decide to "short-circuit" React to handle your component's rendering?

[leanne-tite-jg]

To answer the questions:

• InputPrefix is a plain OOJS class - there is no render method
• Yes - the class pasted in is imported into the component as DNAInputPrefix so they are the same thing
• We have a large number of microsites and need to maximise code re-use and keep our code base DRY. Not all of our products are built with React (in fact the majority are not). Our UI components are built to be agnostic of framework and as dependency-free as possible. We could make React versions of all our components, but then we would have two versions of each, and then the next new library or framework comes along and our react components are no useless etc.... so we have intentionally kept these to vanilla JS

[JakeSidSmith]

Ah, that's a tough one. Would like to help if I can though.

I'd recommend returning false from the shouldComponentUpdate method of the wrapper that creates the InputPrefix instance. If there is other content rendered within this component, I'd recommend creating a separate wrapper for InputPrefix. This way it wont affect wether other React components are rendered, and can easily be added with:

<InputPrefix element="this.refs.element" />

If returning false from shouldComponentUpdate you'll need to move your call to updatePrefix into the componentWillReceiveProps method, which is near enough the same as componentDidUpdate, but should still be called even when returning false.

Also, I'm not sure that setting this.inputPrefix === null; in componentWillUnmount will clean up the InputPrefix correctly. It might be worth adding a destroy method & cleaning up any nodes / listeners manually.

Other than those, all I can suggest is digging through the debugger to see if you can find anything that is being called repeatedly. Or, if the prefix / other components are updating late due to the React components not updating until a request has completed, or something similar.

[leanne-tite-jg]

I will update the class to include a destroy method and experiment with your suggestions on handling the updates. Ultimately we might opt to make React versions of these later. Thanks for all your help

[JakeSidSmith]

No problem. I'm going to leave the issue open in case it does turn out to be react-fastclick related, but from the info you've provided so far, it does not seem to be.

Let me know, here, if you manage to solve this. Then I can close this issue, and as a side, it'd be nice to know how to fix such a problem. :)