User tests: Successful: Unsuccessful:
Improve the PR #9908 (remove modal html from default view, to create body and footer layouts for each modals, and to load each modal with its unique DOM identifier).
Layouts change:
Example of "New File" modal with the new scrolling in list of folders and new display for form fields:
Before Patch :
Form fields to create a file disappear from view on scroll.
After Patch :
Form don't move with scrolling (stay at top), inline form for new file name...
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Components UI/UX |
Also, you are deleting 2 files and you do not modify script.php (forget if the files were added in 3.5.2)
Yes, the files were created and merged in 3.5.2-dev ;-)
In the first PR, i just created the modals body and footer as layouts, and used the existed names for modal identifier.
I did here the change for consistency, and have a logical-related name for identifier of each modal (it was already good for others, only copy modal was identified as "collapseModal" which is in fact used as identifier only for batch modals today (maybe another PR to replace all collapseModal by batchModal when used as so...)
I get again:
About tooltip, what is exactly the problem ?
What i did was to use the same BS HTML strucuture as used in all Joomla forms.
If you do as Batch (not good in my opinion), you will have the tooltip opened and centered on all the div width, not on the label.
This gives a strange tooltip placement in batch modals (and the more your screen is wide, the more it's weird) :
Example with Batch Modal, with tooltip not placed on top of the label.
In fact, this change is to have exactly the same rendering as the renderField
layout : https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/form/renderfield.php
We can use directly the layout here (maybe a not too bad idea?, as this way the display is control by the layout renderField...) to render the field:
JLayoutHelper::render('joomla.form.renderfield', array('input' => $input, 'label' => $label, 'options' => ''));
Indead, this layout renderfield
is not really used today in view, but maybe because views were created before this layout... (@phproberto may help in decision here with his advice, if switch to the joomla.form.renderfield or to keep full html structure inside the view here, as it is now ? ;-) )
In Batch modal, the fields are not rendered the good way, and the span6 was not used for the tooltip on label, but for the fields in 2 columns (another PR i will think to do, to use the renderField layout for each field of batch, as each one are already layout fields).
If you mean the fact the tooltip go out the modal (left), unfortunately, we face here the limitation of the tooltip with no auto placement depending on the container. (i think it's improved in last version of bootstrap, and a bit better in BS3, but BS2 is limited)
I have tested this item successfully on c8cd1f9
If you mean the fact the tooltip go out the modal (left), unfortunately, we face here the limitation of the tooltip with no auto placement depending on the container. (i think it's improved in last version of bootstrap, and a bit better in BS3, but BS2 is limited)
Yes I mean this. I know why the span6 was used in the batch modals. Until we get BS2 or whatever version, adding the span6 for this specific modal looked like the only solution to make this tip readable.
But, if one reduces the screen, we get it nice and clear:
Would be great to get such a result on a monitor.
@infograf768 What is missing is top-left
, top-right
, bottom-left
, bottom-right
... position in Boostrap 2 tooltip.
The fact is in current bootstrap.js, tooltip in top and bottom placement are always centered on the container.
So, in this PR, the placement of the tooltip on the label (and not on a grid span6 element) is the good one (and it's still bad way for batch modals, as not set to label...).
But i agree definitely that a top-left placement (top-right on RTL) for tooltip when inside a modal would be the real solution for this, but not related directly to this PR.
So, i think this PR, could be RTC if you think everything else is ok (as already 2 successfull tests) and in all cases the result is better than in 3.5.1 (where tooltip was behind the modal!).
About a top-left placement when a .modalTooltip
is used, this could be achieve in 2 ways:
$script[] = " $('.modalTooltip').mouseover(function(){";
$script[] = " var left = parseInt($('.tooltip').css('left'));";
$script[] = " var labelWidth = parseInt($('.modalTooltip').outerWidth(true));";
$script[] = " var leftTop = left - (left-labelWidth);";
$script[] = " if ((left-labelWidth) > 0){";
$script[] = " $('.tooltip').css('left', leftTop + 'px');";
$script[] = " } else {";
$script[] = " $('.tooltip').css('left', '0');";
$script[] = " }";
$script[] = " $('.tooltip-arrow').css('left', '25px');";
$script[] = " });";
This code (to be added for quick test, just before the last $script[]
line (not in bs.modal functions) is just a first attempt i've done this evening (so, not perfect...), and you will see that changing the file this way, it will works just now in this PR as well as in all batch modals ;-)
But concerning this PR, the goal was a better HTML rendering, and consistency about modal identifiers.
And as for this reason, i have renamed the 2 files for copyModal, could be good too to add the 3.5.2
label, to not have to edit script to remove files already merged in 3.5.2-dev ;-)
Thanks!
@infograf768 i can create a PR to integrate this tooltip extension for bs2 which works like a charm : https://github.com/andresgutgon/bootstrap-tooltip-extension/blob/master/README.md
And then, add only a minor change to modal layout, to allow a top-left position in all modals ;-)
Now, the question would be if this could be accepted by PLT to add this tooltip extension for additionnal placements... (note: this is full B/C ;-) )
Status | Pending | ⇒ | Ready to Commit |
Milestone |
Added: |
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-04-29 11:01:42 |
Closed_By | ⇒ | Kubik-Rubik |
Labels |
Removed:
?
|
Looks good, thank you @JoomliC. I would also like to see your improvement for the tooltips in another PR! :-)
Great!
So, will do this PR maybe in the afternoon (asap).
Just wonder, if i edit bootstrap.js to integrate tooltip extension, is there a recommendation on the compression tool to be used for min.js ?
Just wonder, if i edit bootstrap.js to integrate tooltip extension, is there a recommendation on the compression tool to be used for min.js ?
No, just take a tool you prefer and which does the job right! :-)
Milestone |
Removed: |
Milestone |
Added: |
Milestone |
Added: |
Milestone |
Removed: |
@Kubik-Rubik So, in fact, i created a full BS tooltip extended plugin (working both on BS2 and BS3), as the one mentionned above was not updated, with issues, and not maintained since 3 years.
And i open just now the PR to introduce extra tooltip positions + RTL support with the auto-dir attribute : #10237
@infograf768 i think now, what you whished as tooltip placement in modal would be possible ;-)
I have tested this item successfully on c8cd1f9
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10100.