User tests: Successful: Unsuccessful:
Pull Request for Issue #10070 .
Based on idea by @RoterNagel about using viewport units for modals, and the proposal of @andrepereiradasilva to create special classes for viewport units.
This PR is full B/C as this one is not changing current behavior. It adds new optional options modalWidth and bodyHeight to be able to set viewport units in the modal constructor.
Note: for old browsers with no viewport unit support, it will just displayed the modal as before.
The Options for modals are then these ones:
title (string) : The modal titlebackdrop (mixed) : A boolean select if a modal-backdrop element should be included (default = true)
The string 'static' includes a backdrop which doesn't close the modal on click.keyboard (boolean) : Closes the modal when escape key is pressed (default = true)closeButton (boolean) : Display modal close button (default = true)animation (boolean) : Fade in from the top of the page (default = true)footer (string) : Optional markup for the modal footerurl (string) : URL of a resource to be inserted as an iframe inside the modal bodyheight (string) : height of the iframe containing the remote resourcewidth (string) : width of the iframe containing the remote resourcebodyHeight (int) : Optional height of the modal body in viewport units (vh)
modalWidth (int) : Optional width of the modal in viewport units (vh)
modalWidth and bodyHeight
jviewport classes to modals.joomla.less
responsive-767px-max.joomla.less
menuTypeModal for test and demonstrate B/CNote: i added viewport options here only for Menu Item Type select for testing purpose. If this PR is accepted, i will then be able to add this to other modals (already updated and merged, and the ones i'm working on and not yet improved). My goal is all modals reviewed for 3.6.0 ;-)
staging
bodyHeight 70vh) as expected in PR #10070 modalWidth to 80vw as the default modal width was already 80% (and in this modal menu-type, seems to be a good width)| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
| Rel_Number | 0 | ⇒ | 10070 |
| Relation Type | ⇒ | Pull Request for |
| Category | ⇒ | Components UI/UX |
BTW also tested in Chrome and IE10
@andrepereiradasilva Thanks for testing and feedback!
And as you can see, i've added too a width option for modal!
Note: for old browsers with no viewport unit support, it will just displayed the modal as before ;-)
(so full B/C)
@brianteeman please add the 3.6.0 milestone here as we have it on the old PR.
@zero-24 no need to remind me on each one. I will review them all a few
times a day.
On 10 May 2016 6:14 pm, "zero-24" notifications@github.com wrote:
@brianteeman https://github.com/brianteeman please add the 3.6.0
milestone here as we have it on the old PR.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10388 (comment)
Thanks! @brianteeman
I have tested this item
successfully on 545a9a3
And you choose the menu type modal for testing
Should I run a test via browserstack, too?
I have tested this item
successfully on 545a9a3
thanks
| Status | Pending | ⇒ | Ready to Commit |
RTC based on tests
| Labels |
Added:
?
|
||
| Milestone |
Added: |
||
@RoterNagel i didn't know what browserstack is, but now i know! ;-)
You can of course!
Thanks for testing!
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-10 17:51:24 |
| Closed_By | ⇒ | rdeutz |
| Labels |
Removed:
?
|
||
Thanks @RoterNagel @andrepereiradasilva and @MATsxm for testing!
Thanks @dgt41 ! ;-)
Now, time to set it for many modals where needed! (just opened 2 PRs for 3 modals, other to come in the next days)
I have tested this item
successfully on 545a9a3
Works as described.
Tested the new menu item type select modal with different window sizes and worked very good.
Tested the other modals (batch, my setting in messages, versions, article select, user notes, user select) and they work as before.
A very nice improvement!
+1 for this modals review for 3.6.0
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10388.