Skip to content
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

Added drag and drop functionality to the item ordering of featured articles (Ref #8138) #9564

Closed
wants to merge 6 commits into from

Conversation

nishadi
Copy link

@nishadi nishadi commented Mar 24, 2016

Pull Request for Issue #8138

Summary of Changes

Added ordering Heading button
Added sort-able drag and drop button with drag and drop functionality
Added save ordering functionality to save the ordering when the items are sorted with drag and drop

Testing Instructions

  1. Go to Back end > Content > Articles
  2. Can sort the article items by drag and drop
  3. Go to Back end > Content > Featured Articles
  4. Can not sort the article items by drag and drop
  5. Apply the patch
  6. Go to Back end > Content > Featured Articles
  7. Now the view shows the drag and drop functionality

@brianteeman
Copy link
Contributor

This seems to work but it is a little confusing (to me at least) as the number doesnt change on screen until after a page refresh


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

JHtml::_('sortablelist.sortable', 'articleList', 'adminForm', strtolower($listDirn), $saveOrderingUrl);
}

$assoc = JLanguageAssociations::isEnabled();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi. why is this ($assoc = JLanguageAssociations::isEnabled();) added here?

@lunalars
Copy link
Contributor

I think the column "Ordering" should be removed completely (like at articles).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@nishadi
Copy link
Author

nishadi commented Mar 25, 2016

I will update the PR according to the given feedback.

@JoshJourney
Copy link

Subscribed. Let us know when this PR is updated.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@Webdongle
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on 8da42a6

Clicking the 'Ordering' column changes the sort by field correctly. But clicking the 'arrow' column doesn't change the sort by field correctly which prevents the drag/drop from working.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @Webdongle


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@nishadi
Copy link
Author

nishadi commented Mar 25, 2016

I have updated the PR removing the ordering column as suggested and now the featured article listing looks exactly like the article listing.
Need clarifications on updating and normalization of the default ordering in featured articles.

Thanks

@Webdongle
Copy link
Contributor

I have tested this item ✅ successfully on 1c6d138

Excellent. Acts exactly the same as the Articles page. Front end Featured Articles displays according to order in the featured drag/drop screen.

Full Success


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@lunalars
Copy link
Contributor

I have tested this item ✅ successfully on 1c6d138

Yup, as webdongle said.
Thanks!


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@andrepereiradasilva
Copy link
Contributor

@nishadi it seems fine now. thanks for your contributions.

I would make two modifications:

  • remove the sortable-group-id="<?php echo $item->catid; ?>" to allow to order across multiple categories. We are talking here about feature articles that can be across categories, right?
  • In the list filter xml i would put the fp.ordering ordering options in first place because it's the first column. That would normalize with the other table list view.

UPDATE see the 2 patches i made to your repository, if all ok for you, accept them.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @lunalars, @Webdongle


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

ordering options order like columns order
@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @lunalars, @Webdongle


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@nishadi
Copy link
Author

nishadi commented Mar 26, 2016

@andrepereiradasilva
Thanks you for the PRs and clarifications. I have accepted them.

@andrepereiradasilva
Copy link
Contributor

@Webdongle sorry? my patch only removed the limitation do order between multiple categories, as it should be. Before the patch has limited to order with drag and drop in one category, after the patch you can order with drag and drop between several categories.

See ab1d799

Are you testing with this settings in the feature menu item?
image

@Webdongle
Copy link
Contributor

@andrepereiradasilva when I tested before your pr was added the articles displayed correctly in the front end. It could have been the removal of the 'Ordering' column'. If @nishadi checks by removing each edit and setting the menu item 1 column then that should show which broke it

Also point of testing etiquette ... although it;s not your patch you have added to it. Therefore should your test results in the tracker be discounted ? I mean no disrespect by that question.

@andrepereiradasilva
Copy link
Contributor

@andrepereiradasilva when I tested before your pr was added the articles displayed correctly in the front end. It could have been the removal of the 'Ordering' column'. If @nishadi checks by removing each edit and setting the menu item 1 column then that should show which broke it

ok, so no problem with my patch them.

Also point of testing etiquette ... although it;s not your patch you have added to it. Therefore should your test results in the tracker be discounted ? I mean no disrespect by that question.

Actually it makes perfectly sense. If i contributed to the PR i shouldn't mark test results. Thank you for the warning. I removed my test result.
I guess i need some lessons of "testing etiquette" 😉

@Webdongle
Copy link
Contributor

@andrepereiradasilva
settings 01

settings 02

settings 03

Same result with
settings 04

There is a Category order in the menu item edit screen and imho removing sortable-group-id="catid; ?>" messed up the way the Component reads the menu item settings.

Thae Category order Setting in the menu item edit screen may be redundant (I have yet to test) but methinks it is required. Because the webmaster may wish to display Category order first then other order per category.
e.g.

Cats
--- Moggies
--- Persians

Dogs
--- Alsations
--- Terrier

Therefore removing sortable-group-id="catid; ?>" breaks the usability in any case. And that change should be reverted.

This is not a 'knock' at your efforts. ... they are appreciated. Just that I test/post in a 'matter of fact' way without emotion.

@andrepereiradasilva
Copy link
Contributor

@Webdongle
Check your backend column order (you are ordering by title there). You should be ordering by order.

image

image

image

@andrepereiradasilva
Copy link
Contributor

@Webdongle

There is a Category order in the menu item edit screen and imho removing sortable-group-id="catid; ?>" messed up the way the Component reads the menu item settings.

That "Category order" option is for, when you have feature articles in several categories, order by that "Category order" first and then by "Article order" (the next field).

You can see that behaviour in https://github.com/joomla/joomla-cms/blob/staging/components/com_content/models/featured.php#L124-L130

My change does not change that. In fact, i made the change to allow the use of "Category order = No order" when you have feature articles in multiple categories, like it was before this PR.

@Webdongle
Copy link
Contributor

I have tested this item ✅ successfully on 75cba61

Retested and Full Success


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@Webdongle
Copy link
Contributor

@andrepereiradasilva

Yep ... that was it had wrong sort column. Have deleted my previous test result and added a successful one. Thanks for finding my PEBKAC

I know what the "Category order" option is for. Had to have breakfast before I had chance to test. I changed the category of one of the featured articles ... applying the patch had no adverse affect.

@andrepereiradasilva
Copy link
Contributor

@lunalars can you test this again after latest changes so it has to successfully tests.

@lunalars
Copy link
Contributor

I have tested this item ✅ successfully on 75cba61

Tested on staging with test content: changed ordering is saved and articles are displayed accordingly.

(And sorry for being late)


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@nishadi
Copy link
Author

nishadi commented Mar 30, 2016

Thanks for testing the PR

@brianteeman
Copy link
Contributor

RTC - thanks


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9564.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 31, 2016
@rdeutz rdeutz added this to the Joomla! 3.6.0 milestone Apr 13, 2016
@rdeutz
Copy link
Contributor

rdeutz commented May 2, 2016

@nishadi cloud you please check the merge conflicts, thanks

@Kubik-Rubik
Copy link
Member

Thank you for creating this. It’s been some time since you created this and there are now some merge conflicts that prevent a direct merge. To save time I have created a new Pull Request from your code and am closing this here.

Check #10313

@Kubik-Rubik Kubik-Rubik closed this May 8, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 8, 2016
@brianteeman brianteeman removed this from the Joomla 3.6.0 milestone May 9, 2016
rdeutz pushed a commit that referenced this pull request May 9, 2016
… (#10313)

* Added drag and drop functionality to the item ordering of featured articles

* Removed ordering column from featured articles

* ordering options order like columns order

* allow sorting across categories

* Merge branch 'staging' of https://github.com/nishadi/joomla-cms into nishadi-staging

Fixed Conflicts:
#	administrator/components/com_content/views/featured/tmpl/default.php

* Updated with changes from #10062
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants