search

markocupic / contao-news-infinite-scroll-bundle

Contao 4 module: Add Infinite Scroll to the Contao Newslist Module
4 stars 1 forks source link

RFC: extend from \Contao\ModuleNewsList (+ other improvements) #5

Closed fritzmg closed 4 years ago

fritzmg commented 4 years ago

Currently, your newslist module does not extend from \Contao\ModuleNewsList. This leads to problems with the newsListFetchItems hook for example, as other app or extension developers will expect a \Contao\ModuleNewsList object as the passed parameter.

Furthermore, by extending from the existing \Contao\ModuleNewsList, you can reuse the majority of its functionality and only add the functionality that you actually need. Thus if there are any changes or improvements in the contao/news-bundle, your own module will automatically benefit from that.

Also, to make this extension even more compatible with other existing newslist extensions, the palette can be changed via an onload_callback instead. This way, any changes to the original newslist palette will be transported over to the newslist_infinite_scroll palette.

However, this PR also removes the canonical tag functionality. Imho this should be part of a different extension, as this does not really have anything to do with the infinite scrolling functionality? And it should be implemented via a hook instead. Since this a backwards compatibility break, I labelled this PR as RFC, to discuss things further.

fritzmg commented 4 years ago

Well, this was an RFC, you shouldn't have merged it right away 😁. Are you fine with the canonical tag functionality being gone?

markocupic commented 4 years ago

We could implementate the canonical tag issue as an option in the module settings, can't we?

fritzmg commented 4 years ago

Definitely. And then add it via hook (since its gone from the module directly).

markocupic commented 4 years ago

@fritzmg Which hook world you recommend?

fritzmg commented 4 years ago

parseArticles, because there you have the module instance as a third parameter and can thus simply check for

if ($module instanceof \Contao\ModuleNewsreader && …)
markocupic commented 4 years ago

Prevent duplicate content by adding the canonical url into the head, if the correspondent field is selected in the module settings: https://github.com/markocupic/contao-news-infinite-scroll-bundle/commit/59eba56fa4d5c7737a65f1078008dc8687bbb04f