Sliders (text-size, brush width, etc) are not draggable on iPad Safari

daiplusplus commented 4 years ago

Describe the bug

The slider in the tools area (e.g. text size, draw brush size, rotation range, etc) cannot be manipulated at all using touch controls on my iPad - my users have to manually enter a number in the input-box to the side of the slider:

The slider cannot be dragged: attempting to drag the slider "thumb" with my fingers doesn't do anything, though the whole slider does briefly darken when initially tapped.
The slider also cannot be moved by tapping on the slider track/rail.

To Reproduce Steps to reproduce the behavior:

Open https://ui.toast.com/tui-image-editor/ in iOS Safari on an iPad (I'm using a 2018 iPad Pro running iOS 13.3.
Select the text tool in the bottom menubar.
Try to drag the Text size slider with your fingers. It won't move.

Expected behavior

I expect to be able to drag the slider with my fingers and to tap any point on the slider trackbar/rail to make the slider thumb move directly to that location.

Screenshots

Smartphone (please complete the following information):

Device: iPad Pro 2018 (MTHN2LL/A)
OS: iOS 13.3
Browser Safari 13

Additional context

Is there a reason you're using a custom control using scripts and <div> instead of using HTML5's <input type="range" />?

I noticed that when entering a value directly into the input box to the right there is no indication that the input box has focus and the displayed value doesn't update until you press the [Enter] key. I see that this is an <input type="text" />. Please change this to <input type="number" min="0" max="30" /> so that browsers can prevent invalid input with native UX.

Additionally, that <input /> next to the range slider should not wire-up the keyup, keydown, and blur events because those won't be fired by all types of user-interaction (e.g. pasting a value won't work), instead it should only use the input event (don't handle the change event either because that only fires when the input loses focus).

auto-comment[bot] commented 4 years ago

Thank you for raising an issue. We will try and get back to you as soon as possible.

Please make sure you have filled out issue respecting our form in English and given us as much context as possible. If not, the issue will be closed or not replied.

daiplusplus commented 4 years ago

I just wrote this runtime patch for the Tui Image Editor that adds HTML5 range inputs, sets-up event handlers, and hides the original touch-unfriendly-sliders:

To use this, just call setUpHtml5Slider() after the ImageEditor has loaded. In my case my page loads the ImageEditor with the page, so I call it during DOMContentLoaded:

window.addEventListener( 'DOMContentLoaded', function( ev ) {
    setUpHtml5Sliders();
} );

function setUpHtml5Sliders() {

    const rangeControls = [
        window.imageEditor.ui.rotate._els.rotateRange, // NOTE: The element class name has a typo: `tie-ratate-range-value` not "tie-rotate-range-value".
        window.imageEditor.ui.draw  ._els.drawRange,
        window.imageEditor.ui.shape ._els.strokeRange,
        window.imageEditor.ui.text  ._els.textRange
    ];

    rangeControls.forEach( convertTuiRangeToHtml5Range );
}

function convertTuiRangeToHtml5Range( tuiRange ) {
    /* Reminder: This is what the DOM looks like:
<li class="tui-image-editor-newline tui-image-editor-range-wrap">
<label class="range">Range</label>
<div class="tie-draw-range tui-image-editor-range">
<div class="tui-image-editor-virtual-range-bar">
    <div class="tui-image-editor-virtual-range-subbar" style="right: 104.72px;"></div>
    <div class="tui-image-editor-virtual-range-pointer" style="left: 49.28px;"></div>
</div>
<input type="range" min="5" max="30" style="width: 100%; display: inline-block;"><!-- ### This element is added by this function. ### -->
</div>
<input class="tie-draw-range-value tui-image-editor-range-value" value="0">
</li>
    */

    /** @type HTMLDivElement */
    const divWrapper = tuiRange.rangeElement; // `<div class="tie-draw-range tui-image-editor-range">`
    correctRangeLabelText( divWrapper );

    /** @type HTMLInputElement */
    const textBoxInput = tuiRange.rangeInputElement;

    // `valueAsNumber` only returns a number if the input type specifically supports it - and unfortunately `<input type="text" />` (or in this case, undefined) will always return NaN even if it has a "numeric" string value. Boo!

    const rangeInput = document.createElement( 'input' );
    rangeInput.type          = 'range';
    if( rangeInput.type !== 'range' ) return; // Don't do anything if the browser doesn't support `<input type="range" />`.

    rangeInput.min           = tuiRange._min.toString();
    rangeInput.max           = tuiRange._max.toString();
    rangeInput.style.width   = '100%';
    rangeInput.style.display = 'inline-block';

    copyInputNumberValue( /*src*/ textBoxInput, /*dest:*/ rangeInput );

    divWrapper.appendChild( rangeInput );

    // Reminder: 'input' happens "live" whereas 'change' happens only when the element loses focus.
    rangeInput.addEventListener( 'input', function( ev ) {

        /** @type HTMLInputElement */
        const inp = ev.currentTarget;

        // Update the value of the <input type="text" /> and fire its input/change event to cause the image-editor to see the value has changed. This will also cause `tuiRange._value` to be updated as well.
        textBoxInput.value = inp.value;
        textBoxInput.dispatchEvent( new Event( 'blur' ) );
    } );

    // Also reverse-wire-up the textbox input to the slider, so manual updates to the input will update the slider:
    textBoxInput.addEventListener( 'input', function( ev ) {

        copyInputNumberValue( /*src*/ textBoxInput, /*dest:*/ rangeInput );
    } );

    textBoxInput.type = 'number';
    textBoxInput.min  = rangeInput.min;
    textBoxInput.max  = rangeInput.max;

    divWrapper.querySelector( 'div.tui-image-editor-virtual-range-bar' ).style.visibility = 'hidden'; // Hide the original slider track and thumb.
}

/**
    * @param {HTMLDivElement} divWrapper */
function correctRangeLabelText( divWrapper ) {

    if( divWrapper.classList.contains( 'tie-rotate-range' ) ) {
        /** @type HTMLLabelElement */
        const label = divWrapper.parentElement.querySelector( 'label.range' );
        if( label.textContent.trim() === "Range" ) {
            label.textContent = "Rotation angle";
        }
    }
    else if( divWrapper.classList.contains( 'tie-draw-range' ) ) {
        /** @type HTMLLabelElement */
        const label = divWrapper.parentElement.querySelector( 'label.range' );
        if( label.textContent.trim() === "Range" ) {
            label.textContent = "Brush size";
        }
    }
}

/**
 * 
 * @param {HTMLInputElement} src
* @param {HTMLInputElement} dest
 */
function copyInputNumberValue( src, dest ) {

    const valueNum = parseInt( src.value, /*radix:*/ 10 );
    if( isNaN( valueNum ) ) {
        dest.valueAsNumber = parseInt( dest.min );
    }
    else {
        dest.valueAsNumber = valueNum; // don't forget that this won't fire 'input' or 'change' on the rangeInput, thus avoiding an infinite loop, phew!
    }
}

Here's how it looks in Chrome 80 on Windows 10:

While initially it isn't as pretty as the original custom slider, native range sliders can still be styled: https://css-tricks.com/styling-cross-browser-compatible-range-inputs-css/

But with these styles:


/* <input type="range"> styling: */
@supports ( -webkit-appearance: none ) {

    input[type=range] {
        -webkit-appearance: none; /* Hides the slider so that custom slider can be made */
        background: transparent; /* Otherwise white in Chrome */
    }
    input[type=range]:focus {
        outline: none; /* Removes the blue border. You should probably do some kind of focus styling for accessibility reasons though. */
    }

    input[type=range]::-webkit-slider-thumb {
        -webkit-appearance: none;
        border: none;
        height: 16px;
        width: 16px;
        border-radius: 8px;
        background: white;
        cursor: pointer;
        margin: 0;
    }
    input[type=range]::-webkit-slider-runnable-track {
    /* Unfortunately we can't have a thin or 1px tall track because:
    1. If the `-webkit-slider-runnable-track` element has a 1px height it's very difficult for the user to click or tap.
    2. It makes positioning the thumb a pain. */

        width: 100%;
        height: 18px; /* 18px height so users can click anywhere, if it's only a few px tall it's very hard to click on */

        cursor: pointer;
        background-color: #404040;
        border-radius: 9px;
    }
}
@supports ( -moz-appearance: none ) {

    input[type=range] {
        -moz-appearance: none;
        width: 100%;
    }
    input[type=range]:focus {
        outline: none; /* Removes the blue border. You should probably do some kind of focus styling for accessibility reasons though. */
    }

    /* All the same stuff for Firefox */
    input[type=range]::-moz-range-thumb {
        border: none;
        height: 16px;
        width: 16px;
        border-radius: 8px;
        background: white;
        cursor: pointer;
        margin: 0;
    }
    input[type=range]::-moz-range-track {
        width: 100%;
        height: 18px;
        cursor: pointer;
        background-color: #404040;
        border-radius: 9px;
    }
}

It looks like this in Chrome, Firefox, Safari (macOS and iOS), and MS Edge (Chromium-based).

jinwoo-kim-nhn commented 4 years ago

The imageEditor with includeUI option was created without considering the mobile situation. Improvements may be included after internal discussions.

derUli commented 4 years ago

This bug also affects Android tablets.

daiplusplus commented 4 years ago

I received a comment reply earlier today (that seems to have been deleted?) asking me to post the copyInputNumberValue function referenced in my code above.

I have been struggling with this and your solution seemed like a light at the end of the tunnel unless I came accross your function call of copyInputNumberValue( /src/ textBoxInput, /dest:/ rangeInput ). you haven't seemed to have defined it anywhere. Care to tell what that is? Just FYI, I am using this library inside React.

I've updated the above reply to now include the definition of function copyInputNumberValue.

binarytrance commented 4 years ago

@Jehoel yes, i had added the commented. deleted it since i tried removing the function call and it woked all fine. What is it's use anyway?

Also, unrelated, it seems like this library uses relative font size. Basically when I add text the default font size seems to be different for images with different dimensions. Is that something you have noticed?

PS: your comments on your code are great. something i would try and incorportate into my own

reggi49 commented 2 years ago

I know this is not a solution, but it can be an alternative that uses image editor on a smartphone. i added button to increase and decrease the brightness, you can also added some icon to increase and decrease the brightness.

here is my code.

const [brightness, setBrightness] = useState(0);

function brightness

brightnessImage(status,value) {
    const editorInstance = this.editorRef.current.getInstance();  // get canvas 
    if(status === 'up'){
      editorInstance.applyFilter("brightness", {
        brightness: parseInt(Brightness + value, 10) / 255,
      });
      const countBrightness = Brightness + value;
      setBrightness(countBrightness );
    }else if (status === "down") {
      editorInstance.applyFilter("brightness", {
        brightness: parseInt(Brightness - value, 10) / 255,
      });
      const countBrightness = Brightness - value;
      setBrightness(countBrightness );
    }
  }

and trigger button

<a
    title="up"
    href="#"
    onClick={() => brightnessImage("up", 10)}>  -
</a>

<a
    title="down"
    className="btn btn-warning btn-fill"
    href="#"
    onClick={() => brightnessImage("down", 10)}>  -
</a>

Jalliuz commented 1 year ago

I guess the problem is in this piece of code

{
    key: "_changeSlideFinally",
    value: function _changeSlideFinally(event) {
      event.stopPropagation();

      // This is where the bug is:
      if (event.target.className !== 'tui-image-editor-range') {
        return;
      }
      // ... More code
  }
}

The clicked slider element can't have extra classes, the comparison is for exact string and sometimes the element gets extra class like "tie-draw-range". Plus, the check is on event.target and the clicked element can be a sub element of the range slider. So this would solve it:

let bar = event.target.closest('.tui-image-editor-range')
if (!bar) {
  return;
}

My temporary solution is to reset the className on the range element before click gets fired rangeElement.className = 'tui-image-editor-range'

But actually, on top of that you also need to transform the touch event to a 'click' event

    handleTouchStart(event) {
      let rangeEl = event.target.closest('.tui-image-editor-range')
      if (!rangeEl) {
        return
      }
      const touchX = event.touches[0].pageX
      const touchY = event.touches[0].pageY
      const clickEvent = new MouseEvent('click', {
        bubbles: true,
        cancelable: true,
        view: window,
        clientX: touchX,
        clientY: touchY,
        pageX: touchX,
        pageY: touchY,
        screenX: touchX,
        screenY: touchY
      })
      // Fix for bug in Tui-image-editor
      rangeEl.className = 'tui-image-editor-range'
      rangeEl.dispatchEvent(clickEvent)
    },

nhn / tui.image-editor

Sliders (text-size, brush width, etc) are not draggable on iPad Safari #365