RRZE-Webteam / rrze-search

Erweiterung der lokalen Sitesearch von WordPress um flexibel anpassbare weitere Suchen. Basiert auf dem Theme FAU Einrichtungen der Universität Erlangen-Nürnberg
0 stars 0 forks source link

Codequalität #22

Closed xwolfde closed 5 years ago

xwolfde commented 5 years ago

In der nächsten Version will ich im Backend kein HTML4-Code oder TABLELAYOUT mehr sehen.

Sowas geht heutzutage eigentlich nicht mehr: <td style="vertical-align:top">

xwolfde commented 5 years ago

Backend muss auch stets Barrierefrei sein. Javascript-Buttons ohne Fallback ist es nicht

 <td>
                    <a href="javascript:rrze_resource_removal(<?php echo $nextResourceIndex; ?>)"
                       class="button button-primary"><?php echo __('Remove', 'rrze-search'); ?></a>
                </td>
jkphl commented 5 years ago

@xwolfde Inline-Styles: Mea culpa (und internes Missverständnis): Ich wollte übers Backend noch drübergehen und alles ins Stylesheet übertragen und aufräumen, hab den Task aber übersehen.

Buttons: Kannst du das bitte spezifizieren? Abgesehen davon, dass hier ein Button hingehört und kein Link (→ wird umgestellt) wüsste ich nicht, was an einem JavaScript-Trigger nicht barrierefrei wäre (solange er einen accessible name hat und tastaturbedienbar ist)? Geht es dir vielmehr um die Funktionstüchtigkeit ohne JavaScript (Robustheit)? (Falls letzteres, soweit ich mich erinnere hatten wir JavaScript als voraussetzbar festgehalten ...?)

xwolfde commented 5 years ago

Wenn du mir sagst, es geht auch von der Barrierefreiheit, dann glaub ich das. Ich hatte eigentlich immer gedacht, das href="javascript:$" nicht gut sein, wenn man kein JavaScript an hat. In dem Fall würde ich auch sagen: Egal, geht halt nicht anders. Ist eh nur für Admins.

jkphl commented 5 years ago

@xwolfde Barrierefreiheit sehe ich tatsächlich hier kein Problem — ist tastaturbedienbar und hat einen accesible name (es wird "Remove" vorgelesen) — allerdings kann man sich philosophisch streiten, ob es nicht ein Button statt einem Link sein sollte (ich sage "ja", und dann würde das JavaScript in den Click-Handler wandern).

JavaScript / Robustheit: Da kennst du ja meine Grundhaltung, aber JavaScript ist bei der Website ja eh Betriebsvoraussetzung (auf jeden Fall im Frontend, und auch das Backend funktioniert ohne JS nur eingeschränkt). Die Admin-Konfigurationsseiten kann man sicher auch auch JavaScript-frei umsetzen, erhöht aber den Aufwand merklich. Falls das wichtig ist, dann sollten wir einen Enhancement-Task daraus machen!?

xwolfde commented 5 years ago

Ne, mit dem JS kann ich leben. Ich war auch eher irritiert wegen dem Table-Layout.. Es gibt auch Teile im Code, wo die wechselnde Zeilenfarbe via PHP berechnet wird... Hier hatte ich eher eine andere Erwartungshaltung. Aber zwingend notwendig oder Bug ist das nicht

jkphl commented 5 years ago

@xwolfde Tabelle ist meine Schuld, wie gesagt. Da hätte ich drübergehen wollen / sollen, ist aber irgendwie nicht gescheit als Task bei mir aufgeschlagen. Wird nächste Woche gleich erledigt!

xwolfde commented 5 years ago

Na, so wichtig ist das nicht. Das können wir später mit machen, wenn wir die andere Suchengine wollen.

xwolfde commented 5 years ago

offline besprechen