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

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

Open
daiplusplus opened this issue Mar 22, 2020 · 8 comments
Open
Labels
Enhancement Enhance performance or improve usability of original features.

Comments

@daiplusplus
Copy link

daiplusplus commented Mar 22, 2020

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.

image

To Reproduce
Steps to reproduce the behavior:

  1. 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.
  2. Select the text tool in the bottom menubar.
  3. 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
Copy link

auto-comment bot commented Mar 22, 2020

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
Copy link
Author

daiplusplus commented Mar 22, 2020

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:

image

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).

image

@jinwoo-kim-nhn
Copy link
Contributor

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

@jinwoo-kim-nhn jinwoo-kim-nhn added Enhancement Enhance performance or improve usability of original features. and removed Bug labels Mar 27, 2020
@derUli
Copy link

derUli commented Jul 14, 2020

This bug also affects Android tablets.

@daiplusplus
Copy link
Author

daiplusplus commented Oct 9, 2020

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
Copy link

binarytrance commented Oct 11, 2020

@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
Copy link

reggi49 commented Dec 17, 2021

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
Copy link

Jalliuz commented Jul 2, 2023

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)
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Enhance performance or improve usability of original features.
Projects
None yet
Development

No branches or pull requests

6 participants