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.

Sign up for GitHub

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

Jump to bottom

[RTM] Add news sorting ability #13

Closed
wants to merge 5 commits into from
Closed

[RTM] Add news sorting ability #13

wants to merge 5 commits into from

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented Jul 5, 2017

This implements #12

I added it for the newslist as well as the newsarchive.

@leofeyer leofeyer mentioned this pull request Jul 6, 2017
Closed
@leofeyer leofeyer changed the title add news sorting ability [RFC] Add news sorting ability Jul 6, 2017
@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 12, 2017

I have changed the field name to news_sorting, which is shorter and more general (since I also implemented the functionality into the news archive module).

@fritzmg fritzmg mentioned this pull request Jul 13, 2017
Merged
leofeyer
leofeyer reviewed Jul 13, 2017
View reviewed changes
src/Resources/contao/modules/ModuleNewsArchive.php
$arrOptions['order'] = "$t.headline DESC";
break;

case 'sort_random':
Copy link
Member

Choose a reason for hiding this comment

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

Do you really think that randomly sorted news is something anyone needs?

Copy link
Contributor Author

@fritzmg fritzmg Jul 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we needed it a couple of times at least. Currently in three projects as well.

@dmolineus
Copy link
Contributor

Also had projects with the requirement of random sorted news/events. Often is required when a set of highlights are teasered on the homepage.

@leofeyer leofeyer added this to the 4.5.0 milestone Aug 23, 2017
@fritzmg
Copy link
Contributor Author

fritzmg commented Oct 4, 2017

Any further changes required? Otherwise I'd consider this RTM.

@leofeyer
Copy link
Member

leofeyer commented Oct 4, 2017

You should mark it RTM when you think it is ready to merge. 😄

@fritzmg fritzmg changed the title [RFC] Add news sorting ability [RTM] Add news sorting ability Oct 4, 2017
@fritzmg
Copy link
Contributor Author

fritzmg commented Oct 4, 2017

Right 😁

@leofeyer
Copy link
Member

leofeyer commented Oct 23, 2017 •
edited
Loading

I just realized that there is a tl_module.news_order field already, which is currently used in the news archive menu. We should use the field in the news list, too, instead of adding yet another field.

@leofeyer
Copy link
Member

I'll adjust the PR accordingly.

@fritzmg
Copy link
Contributor Author

fritzmg commented Oct 23, 2017

Indeed. I forgot about that as well.

@leofeyer leofeyer mentioned this pull request Oct 23, 2017
Closed
leofeyer pushed a commit that referenced this pull request Oct 23, 2017
@fritzmg @leofeyer
Allow to sort news items by date, headline or randomly (see #13).
0241e9a
@leofeyer
Copy link
Member

Merged in 0241e9a.

@leofeyer leofeyer closed this Oct 23, 2017
@leofeyer leofeyer reopened this Nov 8, 2017
@leofeyer leofeyer closed this Nov 8, 2017
@fritzmg fritzmg mentioned this pull request Jan 11, 2018
Merged
@fritzmg fritzmg deleted the news-sorting branch July 5, 2018 05:39
@xchs xchs mentioned this pull request Feb 23, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leofeyer leofeyer leofeyer left review comments

Assignees

@fritzmg fritzmg

Labels
feature
Projects
None yet
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
3 participants
@fritzmg @dmolineus @leofeyer