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.