[3.3] [Fork] Recent Topics NG

[LukeWCS]

phpbb/chameleon

?

Am Rande, mir war klar, dass es bei unserem Fork Projekt zu Synergieeffekten kommen wird: ich werde die Whitespace Änderungen sukzessive überall übernehmen und wahrscheinlich auch die Toggle Farben von RT. Bei EMP fange ich damit an.

[IMC]

dass es bei unserem Fork Projekt zu Synergieeffekten kommen wird

Hat für mich einen sehr hohen Lernwert. Habe gerade festgestellt das meine Wissensdefizite in Sachen GitHub sehr hoch sind.

Ich habe meine Ergänzungen erst einmal hier abgelegt.
https://github.com/IMC-GER/RecentTopics ... arate-page

Bevor ich sie in unseren Haupt-Branch lege brauche ich eine Entscheidungshilfe.
In der Datei recenttopics_body_separate.html habe ich einige Events eingefügt. Es sind viewforum_... Events. Diese habe ich gewählt da RecentTopics auf der Startseite bei der Foren-Übersicht angezeigt wird. Auf der anderen Seite werden mit RecentTopics Beiträge angezeigt, dann wären vielleicht die viewtopic_... Events richtiger.
Obwohl ich mich erst einmal für die viewforum_... Events entschieden habe, bin ich immer noch etwas unschlüssig.

Wenn ich RecentTopics als Forenübersicht betrachte, müsste ich richtiger weise auch einen Eintrag in die Jumpbox (Gehe zu Menue) machen. Dies habe ich erst einmal nicht gemacht da die separate RecentTopics Seite jederzeit über die NavBar erreichbar ist.

[LukeWCS]

Hi Thorsten

Hmm im Moment ist mir noch nicht völlig klar, was genau du vorhast. Ich dachte ich sehe das spätestens dann, wenn du den Code fertig hast.

Ist denn der neue Branch aktuell testfähig? Wenn ja, kann ich mir das in den nächsten 1-2 Tagen anschauen was du gemacht hast, bevor ich antworte.

[IMC]

Ist denn der neue Branch aktuell testfähig? Wenn ja, kann ich mir das in den nächsten 1-2 Tagen anschauen was du gemacht hast, bevor ich antworte.

Hi Patrick,
der Code funktioniert gut. Ist eigentlich auch fertig. Eine Kleinigkeit ist mir eben noch eingefallen, die ich noch nach pflegen werde. Ändert aber nichts an der Funktion.
Lass dir mit dem Anschauen Zeit.

[LukeWCS]

Hi Thorsten

Lass dir mit dem Anschauen Zeit.

Habe es mir jetzt angeschaut, aber erstmal nur grob. Im Moment grad bisserl zuviele Baustellen.

Okay, bin soweit im Boot. Man hat jetzt zusätzlich die Option RT statt im Index auf einer separaten Seite anzeigen zu können. Das werde ich selber weniger nutzen, aber ich finde die Idee gut und das passt auch gut zum Konzept. So haben Benutzer bei RT eine Option mehr. Flexibilität ist immer gut. Lieber Einstellungen die man nicht braucht, als Einstellungen die man vermisst.

Was die Template Events angeht, jupp, bin bei dir. Ich bin da aber gar nicht unschlüssig: viewforum_... passt, aber viewtopic_... passt gar nicht.

Begründung: Ersteres wird dort verwendet wo ein Forum und damit wo eine Themenliste angezeigt wird. Dagegen wird zweiteres nur innerhalb einer Themenansicht verwendet und das passt definitiv nicht zu RT. RT ist im Prinzip von der Aufgabe her ein virtuelles Forum und damit passen die viewforum_... Events perfekt, weil hier eine Themenliste angezeigt wird. Und das unterscheidet sich eigentlich nicht von einer regulären Forenansicht (Themenliste). Der einzige Unterschied ist nur die virtuelle Natur dieses "Forums", weil hier nicht ein spezifisches Forum angezeigt wird, sondern Themen aus verschiedenen Foren.

Es wäre höchstens die Frage, ob man da nicht RT-eigene Eventnamen vergibt. Das ist jetzt aber nur mal so ein flüchtiger Gedanke und definitiv nichts wo ich gründlich drüber nachgedacht hätte. ^^

Beim ersten Blick nach der Aktivierung von RT hatte ich übrigens einen Parser Fehler. Beim de_x_sie\recenttopics.php Paket fehlt ein Komma im Array.

[IMC]

Hi Patrick

Flexibilität ist immer gut. Lieber Einstellungen die man nicht braucht, als Einstellungen die man vermisst.

Deshalb habe ich auch wieder die Seite mit dem simple_header rein genommen. Auf die 5 Zeilen Code ist es dann auch nicht mehr angekommen. Irgendwer hat das in der Vergangenheit genutzt und möchte es vielleicht weiter nutzen.

Forum und damit passen die viewforum_... Events perfekt, weil hier eine Themenliste angezeigt wird. Und das unterscheidet sich eigentlich nicht von einer regulären Forenansicht (Themenliste).

Dann belasse ich es so.

Es wäre höchstens die Frage, ob man da nicht RT-eigene Eventnamen vergibt. Das ist jetzt aber nur mal so ein flüchtiger Gedanke und definitiv nichts wo ich gründlich drüber nachgedacht hätte.

Ich hatte darüber Nachgedacht. Habe den Gedanken aber schnell wieder verworfen. Wenn es als Forum betrachtet wird, sollten die entsprechenden Events greifen. Ansonsten müssten Entwickler RT in ihren Erweiterungen explizit berücksichtigen.

Beim ersten Blick nach der Aktivierung von RT hatte ich übrigens einen Parser Fehler. Beim de_x_sie\recenttopics.php Paket fehlt ein Komma im Array.

Ich weis auch nicht warum mir solche Flüchtigkeitsfehler immer wieder passieren.

Ich werde Morgen die berichtigte Version in unseren Haupt-Zweig als pl7 übernehmen.

[LukeWCS]

Ich hatte darüber Nachgedacht. Habe den Gedanken aber schnell wieder verworfen. Wenn es als Forum betrachtet wird, sollten die entsprechenden Events greifen. Ansonsten müssten Entwickler RT in ihren Erweiterungen explizit berücksichtigen.

Jupp, erstmal lassen. Kann man immer noch anpassen, wenn es dafür einen Grund gibt.

Ich werde Morgen die berichtigte Version in unseren Haupt-Zweig als pl7 übernehmen.

Roger.

Übrigens, wir haben jetzt bereits unseren Rahmen verlassen, den wir gesteckt hatten. Für die neue Option wird zwar eine bestehende Config-Variable benutzt, aber deren Wertebereich liegt jetzt ausserhalb vom Original. Einen Fehler hat das aber nicht zur Folge, wenn man vom Fork auf Original zurück wechselt, weil für user_rt_location und rt_location kein numerischer, sondern ein alphanumerischer Key verwendet wird. Das hat aber zumindest zur Folge, das bei einem "theoretischen" Wechsel zurück auf das Original, diejenigen Benutzer erstmal keine RT Ansicht mehr im Index haben, die beim Fork die neue Option RT_SEPARAT aktiviert hatten. Kann man zwar lösen, indem die Benutzer das im UCP nachbessern, oder der Admin die User Standard überschreibt, aber es ist eben trotzdem eine kleine Inkompatibilität.

[IMC]

Übrigens, wir haben jetzt bereits unseren Rahmen verlassen, den wir gesteckt hatten. Für die neue Option wird zwar eine bestehende Config-Variable benutzt, ... aber es ist eben trotzdem eine kleine Inkompatibilität.

Das habe ich etwas locker gesehen. Für mich war an der Stelle wichtig das keine Migrationsänderung stattfindet und dadurch ein zurück ohne Ausnahmefehler möglich ist.

Ich habe eben RecentTopics durch den EC geschickt und folgende Warnmeldung bekommen:
Warning: Using exit on line 126 in /paybas/recenttopics/controller/page_controller.php

Ich könnte exit() aus den Code entfernen, finde es aber mit sauberer gelöst. Es ist auch nur eine Warnmeldung mit der ich an der Stelle kein Problem habe. Ansonsten bin ich immer bemüht auch Warnmeldungen zu beseitigen.

[LukeWCS]

Das habe ich etwas locker gesehen. Für mich war an der Stelle wichtig das keine Migrationsänderung stattfindet und dadurch ein zurück ohne Ausnahmefehler möglich ist.

Das ist gegeben. Ein Fatal konnte ich an keiner Stelle ausmachen.

Ich könnte exit() aus den Code entfernen, finde es aber mit sauberer gelöst. Es ist auch nur eine Warnmeldung mit der ich an der Stelle kein Problem habe. Ansonsten bin ich immer bemüht auch Warnmeldungen zu beseitigen.

Mit Controller habe ich mich noch nicht wirklich beschäftigt, aber so wie ich das sehe, sollte der default Zweig doch eigentlich auf die Startseite von phpBB leiten, wenn man nach app.php/rt/ etwas angibt, dass der Controller nicht kennt, oder?

Wenn ja, dann klappt das momentan noch nicht, das führt zu einem No route found for "GET /forum33/index.php".

Aber wenn ja, dann könnte man das anders lösen und ebenfalls auf die Startseite weiterleiten, ohne dazu exit() direkt notieren zu müssen. Das können wir phpBB selber überlassen und dann hat EPV auch nichts mehr zu meckern. Das hängt aber davon ab, ob ich deine Absicht richtig verstanden hatte.

[chris1278]

Wenn ihr für phpbb eine alternative für exit sucht geht auch das hier:

Code: Alles auswählen

exit_handler();

Dies hab ich bei meiner DSGVO Ext genutzt.

Wegen Routing solltet ihr den code so benutzen:

Code: Alles auswählen

paybas_recenttopics_page_controller:
    path: /rt/
    defaults:  
        _controller: 'paybas.recenttopics.controller.page_controller:display'