[3.3] Recent Topics NG

[LukeWCS]

Nur auf die Schnelle, komme erst heute Abend zu den Details.

Wenn Dir und mir nichts mehr auffällt, was geändert werden muss,

Ich hab gestern mal angefangen zumindest den Twig Code zu sichten und da ist noch Handlungsbedarf. Ich habe weiteren Code entdeckt, der bei RT anscheinend mit heisser Nadel huschhusch gestrickt wurde. Erstaunlich ist da, dass das "irgendwie" trotzdem funktioniert.

[IMC]

... mit heisser Nadel huschhusch gestrickt wurde. Erstaunlich ist da, dass das "irgendwie" trotzdem funktioniert.

Hoffentlich kein Code von mir. Ich bin gespannt und lerne gern dazu.

[LukeWCS]

Nee, wenn es unseren eigenen Code beträfe, würde ich explizit RTNG angeben.

Diese Datei gibt mir Rätsel auf und über die sollten wir plauschen, bevor wir da was ändern: imcger\recenttopicsng\styles\all\template\event\index_body_markforums_after.html

Code: Alles auswählen

{% if RTNG_DISPLAY && rtng_included != true %}
	{% set rtng_included = true %}
	{% if S_RTNG_LOCATION_TOP %}

		<div id="rtng-stop">
		{% include '@imcger_recenttopicsng/rtng_body_topbottom.html' %}
		</div>
	{% elseif S_RTNG_LOCATION_SIDE %}
		<div id="phpbbsection">
		<div class="index-right">
		<div id="rtng-side">
		{% include '@imcger_recenttopicsng/rtng_body_side.html' %}
		</div>
		{% EVENT recenttopics_mchat_side %}

		{% if ADS_INDEX_CODE && !S_IS_BOT && definition.ADSIDE == false %}
		{% set ADSIDE = 1 %}
		<div class="misc-block advertisement">{{ ADS_INDEX_CODE }}</div>
		{{ definition.ADS }}
		{% endif %}

		</div>
		<div class="index-left">
	{% endif %}
{% endif %}

1.

Code: Alles auswählen

{% if RTNG_DISPLAY && rtng_included != true %}
	{% set rtng_included = true %}

Hier sollte wohl ein Included Flag realisiert werden. Das kann so jedoch gar nicht funktionieren, weil lokale Variablen nur in der Instanz sichtbar sind, die gerade ausgeführt wird. Sollte eine weitere Instanz davon ausgfeführt werden, "weiss" diese nichts von der ersten und somit ist rtng_included immer false und damit ist diese Abfrage komplett sinnfrei. Ich schrieb "sollte", denn mir ist nicht klar, warum hier überhaupt eine solche Abfrage existiert, weil index_body_markforums_after exakt nur einmal ausgeführt wird. Einzig mögliche Erklärung: die Includes könnten von einer anderen Ext ausgeführt werden, aber dann müsste man hier mit globalen Variablen arbeiten und nicht mit lokalen.

2. Der ganze elseif Block ist sehr "seltsam", um es mal vorsichtig zu formulieren.

a. Zum einen fehlen bei den div`s <div id="phpbbsection"> und <div class="index-left"> die schliessenden Tags, was für sich gesehen schon mal ein Fehler ist und ausserdem dafür sorgt, das man hier nicht erkennen kann, wie die Verschachtelungsstruktur tatsächlich aussehen "soll".
b. Dazu gesellt sich noch der Umstand, dass man auch keine Einrückungsebenen eingezogen hat, was das Verständnis noch weiter erschwert.
c. Dann folgt Code für ein weiteres bedingtes Include bzw. eine bedingte Anzeige einer externen Template Variable. Abgefragt wird dazu die globale Variable ADSIDE, aber gesetzt wird die lokale Variable gleichen Namens. Das ist kompletter Nonsens und bringt genau gar nichts.

Dieser Block sieht für mich übrigens so aus wie eine Schnittstelle zu einer anderen Ext, mit der man Werbung anzeigen lassen kann.

Davon abgesehen, in dem Zustand können wir das keinesfalls lassen, das ist Kraut&Rüben und voll mit Fehlern. Wäre ich Validator, würde ich diese Ext alleine für dieses eine Template direkt ablehnen.

[IMC]

Nee, wenn es unseren eigenen Code beträfe, würde ich explizit RTNG angeben.

Das beruhigt mich schon einmal.

Dieser Block sieht für mich übrigens so aus wie eine Schnittstelle zu einer anderen Ext, mit der man Werbung anzeigen lassen kann.

Dazu habe das hier gefunden. Gehört zu einem alten Style von PayBas.
https://github.com/PayBas/PBWoW3ext/blo ... w.php#L175

Zum einen fehlen bei den div`s <div id="phpbbsection"> und <div class="index-left"> die schliessenden Tags,

Die Schließenden Tags sind in der index_body_forumlist_body_after.html. Dort taucht auch wieder das rtng_included auf.

Code: Alles auswählen

{% if RTNG_DISPLAY && rtng_included != true %}
	{% set rtng_included = true %}
	{% if S_RTNG_LOCATION_BOTTOM %}
		<div id="rtng-bottom">
		{% include '@imcger_recenttopicsng/rtng_body_topbottom.html' %}
		</div>
	{% elseif S_RTNG_LOCATION_SIDE %}
		</div>
		</div>
	{% endif %}
{% endif %}

Templates gehören nicht zu meiner Wohlfühlzone. Bisher war ich froh das die Templates funktionieren. Deshalb habe ich mich in der Vergangenheit nur mit dem Template-Code an den Stellen auseinander gesetzt bei denen es notwendig war.

Da hast du einen Bereich identifiziert, in dem ein größerer Optimierungsbedarf besteht. Ich werde versuchen, mich parallel zu dir in das Template zu vertiefen.

[LukeWCS]

Dazu habe das hier gefunden. Gehört zu einem alten Style von PayBas.
https://github.com/PayBas/PBWoW3ext/blo ... w.php#L175

Hmm, da wird eine ganz andere Template Variable genutzt, als bei uns. Okay, im Prinzip aber auch egal. Die Frage ist, ob wir das erlauben wollen. Ich befürchte halt, dass das wieder so ein Relikt ist für eine Ext, die schon längst ABD ist und/oder kaum/keiner mehr danach frägt.

Die Schließenden Tags sind in der index_body_forumlist_body_after.html. Dort taucht auch wieder das rtng_included auf.

Au Backe! Okay, auf diese Idee bin ich jetzt nicht gekommen, die schliessenden Tags in einem anderen Event Template zu suchen. Wenn zwei Exts im Einsatz sind, die beide dieselben Event Templates für öffnende und schliessende Tags nutzen, hat man ruckzuck Salat.

Da hast du einen Bereich identifiziert, in dem ein größerer Optimierungsbedarf besteht. Ich werde versuchen, mich parallel zu dir in das Template zu vertiefen.

Ja und wie wir jetzt im Gespräch festgestellt haben, ein Problem das wir nicht "mal eben" lösen können.

[IMC]

Hmm, da wird eine ganz andere Template Variable genutzt, als bei uns.

Ich sehe dort, und bei uns die ADS_INDEX_CODE. definition.ADSIDE gibt es im zugehörigen Style, PBWoW3style bei avandenberghe (Sajaki).
Für uns ist eigentlich nur von Interesse, dass wir das nicht unterstützen müssen.

Okay, auf diese Idee bin ich jetzt nicht gekommen, die schliessenden Tags in einem anderen Event Template zu suchen.

Ich hätte da jetzt spontan keine andere Idee die seitliche Anzeige zu positionieren. Das nächste umschließende Tag ist das DIV page-body. Da stecken viele andere Elemente drin, die nicht anders positioniert werden sollen.
Deine Bedenken bezüglich der Kompatibilität mit anderen Extensionen kann ich absolut nachvollziehen. Im schlimmsten Fall muss die seitliche Anzeige gehen.

[LukeWCS]

Ah okay, ich hab mich auf die Flag Variable konzentriert, aber die wäre eigentlich gar nicht relevant. Danke für den Stupser in die richtige Richtung.

Für uns ist eigentlich nur von Interesse, dass wir das nicht unterstützen müssen.

Darauf wollte ich hinaus, also das klären.

Deine Bedenken bezüglich der Kompatibilität mit anderen Extensionen kann ich absolut nachvollziehen.

Jupp. phpBB macht es selber zwar auch so wie ich gesehen habe, aber das müssen Exts eben berücksichtigen. Was aber trotzdem passieren kann, dass Exts durch Verteilung der Container Tags auf 2 Events sich gegenseitig beeinflussen können. Wenn z.B. Ext A einen Bereich komplett ausblendet um diesen anders zu generieren und in Event 1 dazu ein öffnendes Tag mit display: none definiert, kann es passieren, das Ext B, die dasselbe Event nutzt um etwas direkt an der Stelle anzuzeigen, von Ext A schlicht "übersteuert" wird und dann die Anzeige von B komplett fehlt. Oder ähnlich gelagerte Probleme.

Im schlimmsten Fall muss die seitliche Anzeige gehen.

Hmm nee, nicht deswegen. Es gilt zwar die potentielle Problematik die ich genannt habe, aber nachdem ich mir das jetzt mal live direkt im HTML Output angesehen habe; es gibt schlicht keine andere Möglichkeit. Jedenfalls sehe ich aus dem Stand keine Alternative. Wenn man das lösen wollte, müsste man das komplett anders aufbauen.

Darum würde ich vorschlagen, wir entfernen die sinnlosen Bedingungen und die besagte Schnittstelle und gut ist. Vielleicht fällt uns da mal später was besseres ein. Dann kannst die Ext wie geplant dieses WE veröffentlichen.

[IMC]

Darum würde ich vorschlagen, wir entfernen die sinnlosen Bedingungen und die besagte Schnittstelle und gut ist.

"Wir" heißt du?

[LukeWCS]

Nicht zwingend, aber kann ich machen. Dann aktualisiere ich gleich noch das switch Makro, das hat noch Stand TC 1.0.0, bei 1.0.1 gabs ne kleine Änderung. Ist aber beim aktuellen phpBB CSS im Prinzip reine Formsache und fällt eher unter Perfektionismus.

[LukeWCS]

b3 online.

Dein Changelog gefällt mir übrigens gut, durch die Kategorisierung ist das prima strukturiert.

Korrektur:

Ist aber beim aktuellen phpBB CSS im Prinzip reine Formsache

Das ist nicht korrekt, wie ich im TC Changelog sah, da hat mir die Erinnering einen Streich gespielt. In so einem Fall zeigt sich mal wieder der Sinn eines Changelogs. Zum Beispiel um auf eigene Falschaussagen aufmerksam zu werden.

* Fix: Im `switch()` Makro wurde bei der Einstellung `checkbox` keine CSS Klasse generiert, wodurch der Mauszeiger über einer Checkbox nicht zu einem Pointer-Cursor geändert wurde. Bei Checkboxen wird jetzt explizit die passende Klasse generiert, die auch im ACP CSS definiert ist.

Setz mal in TC den Schalter Stil auf Checkbox und fahr im RTNG 1.0.0-b2 ACP Modul über eine Checkbox, dann siehst es. Zwar nur ein optischer Fehler, dennoch ein Fehler.

Dann, was die Include Steuerung angeht, ich weiss nicht ob das klar wurde, was ich meinte: Bei einem solchen bedingten Include geht es darum, redundante Inkludierung von HTML, CSS und JS zu verhindern. Bei WWH z.B. gibt es eine solche Steuerung und Kirk sowie chris1278 verwenden diese Steuerung ebenfalls in ihren WWH-kompatiblen Exts. Durch diese Steuerung wird effektiv wiederholtes Inkludieren verhindert, welches entweder zu mehrfachen Darstellungen oder Funktionsstörungen führen könnte.

Durch diese Steuerung kann es z.B. bei Vorhandensein einer solcher WWH-kompatiblen Ext sein, dass die WWH Anzeige gar nicht von WWH selber erzeugt wird, sondern von einer der kompatiblen Exts, weil diese eventuell zuerst die nötigen Komponenten angezeigt hat. Da kann es also passieren, dass bestimmte Templates von WWH Däumchen drehen und selber nüscht zu tun haben. ^^

Für eine solche Ext-übergreifende Steuerung werden zwingend globale Twig Variablen benötigt und genau hier sind die Fehler bei RT gewesen, weil lokale Variablen genutzt wurden.