-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Use ajax power on module order field #11040
Conversation
"position": originalPos | ||
}, | ||
success:function(response) { | ||
console.log(response.data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is just to showcase some working code. it's not commit ready.
Eg $("#jform_position")
the relation with the position field right now is hardcoded, needs an option in the XML for the field moduleorder
Dunno if people use the ordering directly in edit mode, but this is an improvment when there is many module in same position |
@Devportobello from a UX perspective this is the right approach. |
I have tested this item 🔴 unsuccessfully on 4885e07 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
I have applied this PR and try to create "custom" module, I have jQuery error in console. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
I have tested this item 🔴 unsuccessfully on 4885e07 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
I have tested after applying patch with Joomla Version Joomla! 3.6.0-rc Release Candidate [ Noether ] 28-June-2016 20:34 GMT in Windows 8.1, Firefox 47.0.1 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
@RonakParmar @killoltailored Thanks for testing but this is RFC (Request for comments) which means a decision is needed here before I spent more time on this pr. So an up vote or down vote on the scope is enough at the moment (even if the code is not show time ready). Thanks again |
Might have helped if you stated that!
|
@brianteeman I thought that RFC indicator was enough. Maybe I should have opened this as an issue without code attached so people wouldn't be able to do any testing. |
Don't forget that we have a lot of less experienced developers and new
testers.
Never assume anything - when you assume you make a s ASS out of U and ME
|
I am sure they are testing it because they see It isn't should be in a "Needs Review" status? Or may just "New" status and which waiting to be "Confirmed". Thanks. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
Gunman. Unfortunately any PR will show up in patch tester. Patchtester only
shows if it is already RTC or not. Needs Review is an internal flag for CMS
maintainers to make a decision on something.
To be honest a comment that it didn't work is a reply to a request for a
comment in my book :)
|
Great idea. So who are you asking if this is worth it before working on this PR? The folks who manage PR's for new releases of Joomla? Testers? |
@DGT41 - seems the consensus is that yes this is a good idea but that the code currently doesnt work yet This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
Now it should be testable |
Can you remove RFC from title then? |
Not sure if it is doing what I expect it to do This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
@brianteeman before applying this patch was the behaviour different? All this pr is doing is reloading the ordering when the position is changed, it shouldn't interfere with the actual ordering in the front end... |
* | ||
* @note The field does not include an upload mechanism. | ||
* @see JFormFieldMedia | ||
* @since 11.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11.1
? better always to use __DEPLOY_VERSION__
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think __DEPLOY_VERSION__
needs to be on other places too where 3.7
is added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrepereiradasilva @gunjanpatel you are both right but I think __DEPLOY_VERSION__
came after july 6th. Anyways I will fix this..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh sorry didn't noticed dates. Thanks 👍
I have tested this item ✅ successfully on 9ed547e I still dont really see what the difference is now though compared to before this pr This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
I have tested this item ✅ successfully on 9ed547e This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
@DGT41 can you double check the since tags? If that is done i think we can RTC |
@zero-24 there you go! |
* | ||
* @return string The field input markup. | ||
* | ||
* @since 11.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getInput() was there before so the since should not change. (11.1 is wrong should be 1.6)
Hmm i have found two more ;) |
Go for it @joomla-cms-bot please make us happy 😄 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11040. |
Enhance the current status quo
Try to create a new module in module manager. Select a position. Select another position
The dropdown ordering doesn't really represent the true order of the modules in the new position.
So my idea was to use ajax power to be always accurate on that dropdown.
Summary of Changes
Use ajax power, tight connection with the controller module.php
Preview
Mind the untranslated string!
![screen shot 2016-07-06 at 23 23 04](https://cloud.githubusercontent.com/assets/3889375/16633245/05758102-43d1-11e6-80c0-6bcbdd2c1aa6.png)
No other module in that position:
With other modules in the same position:
![screen shot 2016-07-06 at 23 24 48](https://cloud.githubusercontent.com/assets/3889375/16633259/177b2f5a-43d1-11e6-9c3c-d6c5fa834c79.png)
Testing
Please DO NOT TEST this one!
Instead give a vote (up or down thumb) so PLT can make a decision (or PLT decides directly, whatever comes first)