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

[Modal] Scrolling broken for mobile devices from recent 2.3.2 changes #6449

Closed
ngan opened this issue Jun 22, 2018 · 61 comments
Closed

[Modal] Scrolling broken for mobile devices from recent 2.3.2 changes #6449

ngan opened this issue Jun 22, 2018 · 61 comments

Comments

@ngan
Copy link

ngan commented Jun 22, 2018

Steps

This can easily be replicated pressing "Run Code" in the "Scrolling Modal" example on a mobile device (Chrome or Android) or even in a device simulator on Chrome Desktop.

Expected Result

It should be able to scroll.

Actual Result

It cannot scroll.

Version

2.3.2

Testcase

https://semantic-ui.com/modules/modal.html#scrolling-modal

This is the culprit: 75dcaa2

@ngan
Copy link
Author

ngan commented Jun 22, 2018

@jlukic I'm going to do some more investigation on why 75dcaa2 broke scrolling, but I have a suspicion that when you scroll on the modal, the touch event bubbles up to the dimmable and is canceled (via preventDefault).

@y0hami
Copy link
Member

y0hami commented Jun 22, 2018

I can confirm this is broken on

  • Android Chrome
  • Android Firefox
  • iOS Safari 11
  • iOS Safari 10
  • iOS Safari 9
  • iOS Chrome

@ngan
Copy link
Author

ngan commented Jun 23, 2018

I have a suspicion that when you scroll on the modal, the touch event bubbles up to the dimmable and is canceled (via preventDefault)

After further digging, this is exactly what's happening. I'm not exactly sure what the original commit was trying to solve (no issues referenced) so it's hard to suggest how it should be fixed. Without the patched code, I already can't scroll the background on iOS.

@aakashmalviya
Copy link

Another problem is on mobile content inside modal is not scrolling using touch.. Content inside modal scroll only by pressing scroll bar..
Please fix it urgently ........

@aakashmalviya
Copy link

Please provide some codes for temporary fixing of this issue. Please ... URGENT !!!!

@ColinFrick
Copy link

@aakashmalviya You could revert to 2.3.1 until this is fixed

@aakashmalviya
Copy link

aakashmalviya commented Jun 26, 2018

@ColinFrick Okay. Thank you for suggestion. I will do it...

@aakashmalviya
Copy link

The problem is also in popup. Scrolling in popup also not working using touch on mobile.

@nasatome
Copy link

nasatome commented Jun 26, 2018

When reverting to version 2.3.1 it is solved.
It affects pages even if they are not using modal or dimmer windows.

module.unbind.events they are never called; this break all page in touch screen (with the new commit)

unbind: 
  events:

I do not know how semantic-ui works but a quick debugging led me to:
module.destroy (); It is never called

so that
$dimmable.get(0).removeEventListener('touchmove', module.event.preventScroll, { passive: false });
it can not be applied

it's a good idea to mark this bug as a high priority, it practically leaves semantic ui unusable drives with touch (android / ios)

@andygmorris
Copy link

I found this error too today when developing a new website (clean code) with SUI v2.3.2 - Reverting SUI back to v2.3.1 (with jquery v3.1.1 or v3.3.1) resolves the issue. Indeed, should be a High Priority fix as this breaks mobile websites (if people were to use the latest version).

@postbud
Copy link

postbud commented Jul 12, 2018

I confirm the appearance of the problem after last update

@nasatome
Copy link

@jlukic There will be some way to prioritize this problem that leaves semantic-ui not usable for current mobile use

@0angelic0
Copy link

Confirmed.

@smtlab
Copy link

smtlab commented Jul 14, 2018

I can confirm the issue. My website bounce rate increased suddently to 89% 😑 as my sign in modal opens on page load was unaware of this issue. Is there any temporary fix for this issue, apart from downgrading to 2.3.1 ?

Is there anyway I can help with this?
Thanks

@tm4321
Copy link

tm4321 commented Jul 16, 2018

I appreciate Semantic trying to solve the issue with the background scrolling on mobile devices; however, I don't think 75dcaa2 was the correct approach. Personally I think this issue should be left to the developers until there is a simple and universal solution.

I've tackled this problem several times and have solved it in two different ways:

  1. Restrict the body height and structure your HTML so that your modal doesn't propagate the scroll events. Here is a nice blog post that has some working demos as well.

  2. Use JavaScript and CSS to manually lock the body in place and then reset the scroll position once your modal closes. I'm actually using this solution on my personal website in the projects and contact modals. You can view the relevant code here.

Here is the bare minimum code needed:

var SCROLL_TOP

function disableScroll() {
  SCROLL_TOP = $(document).scrollTop()

  $("body").css({
    "position": "fixed",
    "left": "0px",
    "right": "0px",
    "top": `${SCROLL_TOP * -1}px`
  })
}

function enableScroll() {
  $("body").css({
    "position": "",
    "left": "",
    "right": "",
    "top": ""
  })

  $(document).scrollTop(SCROLL_TOP)
}

@batata004
Copy link

batata004 commented Jul 18, 2018

I can confirm this bug happens not only when you have modals opened, it happens even when you dont use modals. In the case of https://www.sitepor500.com.br if you simply open it (dont need to raise/open any modals) the scrolling will not work. I was using 2.3.3. version and then I downgraded it to 2.3.1. and everything came back to work just fine.

@samuelmattos
Copy link

I have same problem.

@cprcrack
Copy link

Is this the reason scrolling on https://semantic-ui.com/kitchen-sink.html doesn't work on mobile devices?

@samuelmattos
Copy link

@cprcrack I do not know but i solved it altering version to 2.2.14

@batata004
Copy link

@samuelmattos you dont need to use such old version, use 2.3.1 and it will be fine.

@cprcrack
Copy link

I'm a little bit alarmed about the fact that this critical bug made into release and even more by Semantic UI team's slow response time to it. I tried to find info about the team behind Semantic UI but couldn't find anything. Could someone enlighten me on this matter and about the expected future of the project?

@batata004
Copy link

@cprcrack I agree with you, this is not an aesthetic bug or comestic stuff. This is a serious bug, I runned an adwords campaign with about 300 dolars on 2 days just to realize I was not having any success because I had updated the semantic ui version and it prevented my users to scroll down and find the contact form. Very nasty bug.

Semantic ui is very nice, is a library well documented and despite many errors in its documentation, it is beautiful and light. However serious mistakes like the one reported in this thread make this library lose lots of its credibility.

@byte916
Copy link
Contributor

byte916 commented Jul 23, 2018

I fix it -> #6495

@smartm0use
Copy link

Hi,
I've upgrade Semantic-UI from 2.2.11 to 2.4.2 and I'm encountering this issue.
How can I fix it?

@smartm0use
Copy link

smartm0use commented Dec 19, 2018

To make it to work again in my Angular app (with Semantic-UI 2.4.2) I had to add the following code:

EventTarget.prototype['addEventListenerBase'] = EventTarget.prototype.addEventListener;
EventTarget.prototype.addEventListener = function(type, listener){
	if(this !== document.querySelector('body') || type !== 'touchmove'){
		this.addEventListenerBase(type, listener);
	}
};

@lubber-de

This comment was marked as spam.

@jjylha
Copy link

jjylha commented Jan 7, 2019

@jlukic @hammy2899

Seriously guys.. why do we need Semantic UI and Fomantic UI? Why couldn't @hammy2899 just fix that to Semantic UI?

I just replaced the Semantic UI 2.4.2 modal.js file with Fomantic UI fixed modal.js file and everything works fine...

@batata004
Copy link

@jjylha good point. Clearly semantic ui is going down hill, no maintanance and bad support. This is terrible for a recent library like semantic ui. It had SO MUCH potential, just wasted for lack of support and updates.

@y0hami
Copy link
Member

y0hami commented Jan 7, 2019

@jjylha @batata004 If you want to find out why FUI was created please read the following https://github.com/fomantic/Fomantic-UI/blob/master/faq.md

@batata004
Copy link

@hammy2899 I know why it was created, I already participated in another discussion and I think Fomantic UI is doing a great job. I just see no reason for both exist if semantic ui adopted all the good practices of fomantic: regular and fast updates.

@y0hami
Copy link
Member

y0hami commented Jan 7, 2019

@batata004 I did have an explanation somewhere but I can't find it on why we couldn't do it here (SUI repo).

tldr: The author/main maintainer has vanished and we haven't been able to contact him for months and we haven't had explicit permission to start merging PR's and shipping releases therefore we created our own repo/org so we could do so.

@batata004
Copy link

@hammy2899 you are doing a great job at fomantic. Just a remember: you can use fomantic at the CDN -> https://www.jsdelivr.com/package/npm/fomantic-ui (it loads very fast).

@jjylha
Copy link

jjylha commented Jan 7, 2019

@hammy2899

I mean the original idea for Fomantic UI is good and understandable in this situation:
"Fomantic was created to continue active development of Semantic-UI and has the intent to be merged back into the master repository once active development can restart. "

But if you are actually planning to make all of these changes we will not be able to change to Fomantic UI anyway: fomantic/Fomantic-UI#319 No resources to refactor everything.

@jlukic

Bro. You should really consider letting more people maintain this stuff....

@y0hami
Copy link
Member

y0hami commented Jan 7, 2019

But if you are actually planning to make all of these changes we will not be able to change to Fomantic UI anyway: fomantic/Fomantic-UI#319 No resources to refactor everything.

I mentioned why we where doing this in that issue.


Can we keep this issue on topic please.

@ivanlysakov
Copy link

Solved this bug wih Fomantic-UI. Thanks guys)

@smartm0use
Copy link

I'm using a forked SUI for my application and the last version is 2.4.2. Can you tell me how to make scrolling with touch working from mobile devices? What to change in the source code of Semantic-UI 2.4.2?

@lubber-de

This comment was marked as spam.

@smartm0use
Copy link

I have already tried to do it, but the files look different. I tried to replace some lines of modal.js anyway, but it doesn't work.

@jjylha
Copy link

jjylha commented May 3, 2019

We just replaced the SUI modal.js with FUI modal.js for now and everything seems to work fine...

@smartm0use
Copy link

smartm0use commented May 3, 2019

I tried to replace SUI modal.js with FUI modal.js, but in my case it doesn't do the trick.
I'm still fixing my touch problems using this workaround.

That workaround have done the trick for some months, but now it is causing another bug, so I have to find another solution... and yours don't work with my application :(

@MrZenW
Copy link

MrZenW commented Dec 2, 2019

To make it to work again in my Angular app (with Semantic-UI 2.4.2) I had to add the following code:

EventTarget.prototype['addEventListenerBase'] = EventTarget.prototype.addEventListener;
EventTarget.prototype.addEventListener = function(type, listener){
	if(this !== document.querySelector('body') || type !== 'touchmove'){
		this.addEventListenerBase(type, listener);
	}
};

It works for our project. Thank you smartm0use, you made me solving this bug without upgrade the UI library, as I don't want to continuously upgrade the dependence version for this UI library in my project. Sometimes the new version fixes one bug but brings new bugs to me. For a commercial project, stability is more important than the new feature.

@lucasdavila
Copy link

I am having a similar issue with semantic 2.4.2.

@lucasdavila
Copy link

lucasdavila commented Jan 29, 2020

I was able to temporally fix it by replacing the .semantic/src/definitions/modules/modal.js file with the one from fomantic https://raw.githubusercontent.com/fomantic/Fomantic-UI/master/src/definitions/modules/modal.js

Then I build semantic again, copied the dist files to my project and it worked.

The fixup is related to this PR https://raw.githubusercontent.com/fomantic/Fomantic-UI/master/src/definitions/modules/modal.js

@lucasdavila
Copy link

I wonder if you guys could work together in the same repo <3.

@RicottaZhang
Copy link

To make it to work again in my Angular app (with Semantic-UI 2.4.2) I had to add the following code:

EventTarget.prototype['addEventListenerBase'] = EventTarget.prototype.addEventListener;
EventTarget.prototype.addEventListener = function(type, listener){
	if(this !== document.querySelector('body') || type !== 'touchmove'){
		this.addEventListenerBase(type, listener);
	}
};

I use this in my project, this fixed mobile modal scrolling, but import no selection for additional selection dropdown.

@lbettuz-lcb
Copy link

To make it to work again in my Angular app (with Semantic-UI 2.4.2) I had to add the following code:

EventTarget.prototype['addEventListenerBase'] = EventTarget.prototype.addEventListener;
EventTarget.prototype.addEventListener = function(type, listener){
	if(this !== document.querySelector('body') || type !== 'touchmove'){
		this.addEventListenerBase(type, listener);
	}
};

I use this in my project, this fixed mobile modal scrolling, but import no selection for additional selection dropdown.

Hello everybody.
Hope you've found your work-around.
Unfortunately, this one won't be ok for me. Those lines creating infinite loop with a specific manipulation.
(opening modal, coming back, reopening modal).
Not sure how to deal with this. Very annoying.
May be my only solution is to downgrade.

@szalailaszlo
Copy link

Hi All!
Quick fix for touchevent stuck: because it added this eventlistener to the contexted element, simple add an empty, invisible element to your html body, and attached the modal for it:

.modal({ ... context: $('#yourInvisibleModalTouchMoveDiv'), ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests