? Success

User tests: Successful: Unsuccessful:

avatar nikosdion
nikosdion
12 Aug 2016

Summary of Changes

Commit 93f90ef had the exact opposite
effect of what was advertised, i.e. it made it impossible to use
visible ordering input fields when using the drag'n'drop reordering.
This has broken –among other things– FOF 2.x which is part of core
Joomla!. It's worth noting that BEFORE this commit it was actually
possible to have visible reordering fields and I was in fact using them
in FOF 2 (which is included in Joomla! itself) since 2014. So, basically,
you guys broke Joomla! because you didn't test that the commit does
what it says it does instead of what the commiter thought his commit
does. TEST. BEFORE. COMMITING!!!

Interestingly, the actual problem @sebastienheraud was trying to solve
was in fact solved by the changes @alexvanniel did in gh-5731 This would
have been all too well if gh-5731 hadn't kept the :hidden specifiers
from commit 93f90ef which had introduced
a regression.

My changes keep the ACTUAL problem's fix from gh-5731 and remove the
regression introduced in 93f90ef

Testing Instructions

I am not providing any on the grounds that you blindly accepted commit
93f90ef and gh-5731 without testing.

Since this fixes FOF 2 which is part of Joomla! I expect you to merge it
anyway.

For what it's worth, I have tested this PR with all of my components using
both FOF 2 and FOF 3. Before the PR drag'n'drop reordering with visible order
fields was impossible: the row would move but the value in the ordering field
remained the same. After the PR drag'n'drop reordering with visible order works
as expected: the row would move AND the value in the ordering field changed.

I also tested with com_content. Before AND after the change you can drag'n'drop
reorder just fine. PLEASE DO TEST WITH CORE COMPONENTS YOURSELVES! We
must be sure that no core component breaks with this change. Also, please do
test with third party components for the same reason. Let's do this the right way.

Documentation Changes Required

None. This PR fixes what you guys had already broken.

avatar joomla-cms-bot joomla-cms-bot - change - 12 Aug 2016
Category JavaScript
avatar nikosdion nikosdion - open - 12 Aug 2016
avatar nikosdion nikosdion - change - 12 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Aug 2016
Labels Added: ?
avatar brianteeman
brianteeman - comment - 12 Aug 2016

Can't test so won't test

avatar nikosdion
nikosdion - comment - 12 Aug 2016

@brianteeman You CAN test it. You can use the FOF 2.x demo component (todo) from https://github.com/akeeba/todo-fof-example/tree/master create a bunch of todo items and try reordering them.

However, I expected you to set it to RTC and commit it without any further ado like you did with the TWO commits that completely screwed up drag and drop reordering. You know, the ones I wasted two hours of my time to track down and fix in this PR because you guys don't care when you're breaking backwards compatibility and only play it hard when someone WHO ACTUALLY KNOWS HOW THIS THING WORKS spends his time fixing what you broke!

Forget it. I'll just remove the feature you stupidly broke from FOF 3 and let you figure out what to do with the end of life FOF 2 you still have in the core. I won't try to fix core bugs again. It's clear now that you will only blindly accept code from idiots who don't know how Joomla! works and break it instead of the people who DO know how it works and try to fix it. My bad. I should have learned by now.

avatar nikosdion nikosdion - change - 12 Aug 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-08-12 16:28:15
Closed_By nikosdion
avatar nikosdion nikosdion - close - 12 Aug 2016
avatar rdeutz
rdeutz - comment - 12 Aug 2016

What's going on here, I am just looking how I can test it.

avatar brianteeman
brianteeman - comment - 12 Aug 2016

@nikosdion I didn't test, merge or mark rtc either of the two pr you refer to.

avatar ggppdk
ggppdk - comment - 12 Aug 2016

I had noticed it did not work with order input fields visible,
and the selector seem strange,

  • but did not look at it any more at that particular time

Tested this fix and it works:


To test with Joomla backend featured manager

  1. isis template add file css/custom.css with:
.text-area-order, .sortable-handler + input {
  display: inline !important;
  width: 40px; text-align: right;
}
  1. In feature manager make hard refresh to get new JS loaded and sort by the ordering the column
  2. E.g. i have 10 records, manually (typing) reverse the last 4 items e.g. 7 - 8 - 9 - 10 to 10 - 9 - 8 - 7
  3. Drag and drop some of the records at the top, this will trigger an AJAX save too, like you clicked a save button, thus your manual ordering will get saved too
  4. DO a CTRL-F5 or similar refresh

You see that the drag and drop ordering worked, and that the records that you manually ordered were saved

To add save button to the header of the column
After:

<?php echo JHtml::_('searchtools.sort', '', 'fp.ordering', $listDirn ...

Add:

<?php echo JHTML::_('grid.order', $this->items, 'filesave.png', 'featured.saveorder' ); ?>

Change some input manually to different order, and click on different field to blur the input,
you will see that the manually save button gets activated,
click it to save

avatar brianteeman
brianteeman - comment - 12 Aug 2016

Thank you for providing a way for this to be tested

avatar ggppdk
ggppdk - comment - 12 Aug 2016

Sorry, i missed a step above:
4. After you drag and drop DO a CTRL-F5 or similar, to reload the view and force correct display of the saved fields

Maybe adding the manual save button is easier for testing (see above)

avatar rdeutz
rdeutz - comment - 12 Aug 2016

In fact #7368 broke it, they are two ways of fixing it. At the moment I am checking what is the best way to fix it and to have the ability to have text fields in sortable tables. PR is on the way

avatar rdeutz
rdeutz - comment - 12 Aug 2016

To see what is not working I used the URL-Redirection from admin-tools-pro, before the patch it doesn't work after it works. Didn't test a FOF3 component

Add a Comment

Login with GitHub to post a comment