? Success

User tests: Successful: Unsuccessful:

avatar JoomliC
JoomliC
17 Feb 2016

Fix scrolling inside bootstrap modals (small height of window viewport: mobile, window resized...).
Corrects the media modal padding on small devices.
Patch for Isis and Protostar templates.

Steps to reproduce the issue

Isis template
In admin test new bootstrap modals introduced in 3.5.0 beta2 :

  • Add New Menu Item : Menu Item Type (component views) or Link Image (image) option
  • Add New Article : Created By (users), Intro Image (image)
  • In list of items : test Batch modals
  • Everywhere a bootstrap modal is used...

Protostar template
In frontend :

  • Edit Article : Versions, Intro Image (image) ...
  • Everywhere a bootstrap modal is used...

Expected result

We should be able to scroll inside the modal, and use all options when using administration on a mobile or small screen device.

Actual result in 3.5.0 beta2

You may not be able to scroll the modal on iOS (iPhone) (tests on other mobile OS are welcome!)
On window resized (small height), modal could be off screen (bottom content not accessible)

BEFORE PATCH on iPhone 4S iOS 9.2.1
iphone4s-before-patch
Modal off screen. Not possible to scroll.

Additional comments

Scrolling not working on mobile is a known issue of Bootstrap modals, not fixed in BS 2 and 3 : twbs/bootstrap#14839 (and maybe related to a webkit bug: https://bugs.webkit.org/show_bug.cgi?id=153856)
To set a max-height for modal-body class depending on the current viewport height solves the issue of not being able to view bottom of the modal window. (resize your window to arround 300px height before and after patch)

Testing

Apply the patch, and you may be able to scroll inside all bootstrap modals.

AFTER PATCH on iPhone 4S iOS 9.2.1
iphone4s-after-patch
Modal adapts to screen. Scroll is working.

Note: Review of the comments by a native English speaker will be appreciated ;-)

avatar JoomliC JoomliC - open - 17 Feb 2016
avatar JoomliC JoomliC - change - 17 Feb 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Feb 2016
Labels Added: ?
avatar dgt41
dgt41 - comment - 17 Feb 2016

works for me! Thanks @JoomliC

avatar dgt41 dgt41 - test_item - 17 Feb 2016 - Tested successfully
avatar dgt41
dgt41 - comment - 17 Feb 2016

I have tested this item :white_check_mark: successfully on 91f2c71


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9140.

avatar rehrumesh
rehrumesh - comment - 17 Feb 2016

I have tested this item :white_check_mark: successfully on 91f2c71

avatar brianteeman brianteeman - change - 17 Feb 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 17 Feb 2016

RTC - thanks


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9140.

avatar joomla-cms-bot joomla-cms-bot - change - 17 Feb 2016
Labels Added: ?
avatar JoomliC
JoomliC - comment - 18 Feb 2016

Thanks @dgt41 and @rehrumesh for testing!

@brianteeman hope my English not too bad in the layout modal main file ;)


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9140.

avatar Kubik-Rubik Kubik-Rubik - change - 18 Feb 2016
Milestone Added:
avatar Kubik-Rubik
Kubik-Rubik - comment - 18 Feb 2016

Thank you @JoomliC and testers!

avatar Kubik-Rubik Kubik-Rubik - close - 18 Feb 2016
avatar joomla-cms-bot joomla-cms-bot - close - 18 Feb 2016
avatar Kubik-Rubik Kubik-Rubik - reference | 95ecb30 - 18 Feb 16
avatar Kubik-Rubik Kubik-Rubik - merge - 18 Feb 2016
avatar Kubik-Rubik Kubik-Rubik - close - 18 Feb 2016
avatar Kubik-Rubik Kubik-Rubik - change - 18 Feb 2016
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2016-02-18 08:36:58
Closed_By Kubik-Rubik
avatar joomla-cms-bot joomla-cms-bot - change - 18 Feb 2016
Labels Removed: ?
avatar rehrumesh rehrumesh - test_item - 23 Feb 2016 - Tested successfully
avatar rehrumesh
rehrumesh - comment - 23 Feb 2016

I have tested this item :white_check_mark: successfully on 91f2c71


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9140.

avatar brianteeman
brianteeman - comment - 23 Feb 2016

@rehrumesh thanks for your tests but you are testing issues that have already been merged


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9140.

avatar rehrumesh
rehrumesh - comment - 23 Feb 2016

@brianteeman earlier I hadn't use the JTracker application. So marked it again using JTracker.

avatar smz
smz - comment - 3 Apr 2016

Sorry to resurrect this already merged PR, but I think what has been done here is really wrong:

You can't fix a Bootstrap modal problem by hacking the content of the modal. That could work today for today backend content but has the potential of breaking frontend usage of the Bootstrap modals or tomorrow usage in the backend with different header/footer sizes.

A correct approach would had been to correct the issue at the modal (container) itself, and absolutely not by imposing restrictions on the .modal-body based on specific assumptions about the header, footer and padding size.

This had probably meant to correct the Bootstrap modal JS itself, but that would had been the only correct solution.

avatar smz
smz - comment - 3 Apr 2016

OMG, now I see this is a specific Webkit issue, and there are correct solutions here: twbs/bootstrap#14839

I think we can't cut corners to badly fix specific iOS issues, especially if correct solutions are already available.

Add a Comment

Login with GitHub to post a comment