GRUD_DEV-1121/feat/add preview center

[TimoGebendorfer]

Submit a pull request

Please make sure the following is true:

• I gave the PR a meaningful name
• I checked that the correct target branch is selected
• I rebased the branch on the target branch and it can be merged
• I ran the linter and it did pass
• I checked for unused code / dead code / debug code
• I checked that variables/functions have meaningful names
• I checked that the behaviour is as the documentation/task describes and I tested it
• I updated the docs / specifications if possible
• I could explain all that code when someone wakes me up at 3am
• I checked that the code considers failures and not just the happy path
• There are no new dependencies OR I listed them and explained them below
• PR introduces no breaking changes OR I listed them and described them below
• I added/updated tests for new/modified unit-testable functions/helpers
• I ran the tests and they did pass

Other information/comments (e.g. reasons why points are not checked from above)

mMn. wurde kein neuer Code hinzugefügt welcher tests benötigt.

New dependency

Dieser PR beinhaltet einen neue dependency namens swipper (https://swiperjs.com/). Sie fungiert als Slider und ist im Projekt als solcher in einer simplen Form im Einsatz.

Reason for this PR

Introduces the new generic preview center of task: https://campudus.myjetbrains.com/youtrack/agiles/112-23/current?issue=GRUD_DEV-1121

[Copilot]

Pull Request Overview

This PR introduces a new generic preview center feature that allows users to preview row data in a dedicated view with side-by-side panels showing row data and detailed column information.

• Adds a complete preview view system with resizable left/right panels
• Implements various cell type renderers for displaying different column types (boolean, currency, text, links, attachments)
• Integrates Swiper.js dependency for image slider functionality in attachment previews

Reviewed Changes

Copilot reviewed 42 out of 45 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/scss/variables.scss	Adds orange color variable for UI styling
src/scss/svgIcon.scss	Adds color classes for success and red SVG icons
src/scss/preview/*.scss	New stylesheet files for preview components styling
src/scss/main.scss	Imports preview stylesheets and chip component
src/locales/*/preview.json	Adds German and English localization strings
src/app/types/grud.ts	Extends type definitions for preview functionality
src/app/redux/	Adds preview reducer, actions, and state management
src/app/components/preview/	Implements all preview view components and helpers
src/app/components/Router.jsx	Adds routing for preview URLs
package.json	Adds Swiper.js dependency

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[hermann-p]

Ich habe mir jetzt mal nur den Code angeschaut. Ist nicht so schlimm wie es aussieht (also, mein Review ist nicht so schlimm wie die Anzahl der Anmerkungen vermuten lässt, Code selber ist ziemlich sauber und gut strukturiert) - an vielen Stellen kann man einfach nur existierende Helper nutzen bzw. die neuen URLs in das File apiHelper.js aufnehmen, so dass die wiederverwendet werden können und man auch sicher ist, immer die korrekte URL zu tippen.

An ein/zwei Stellen habe ich Fragen, die sind auch als Fragen gemeint.

Mir ist nicht ganz klar, wie row.values im Zusammenhang mit den Displaywerten hier verwendet werden…

So als allgemeiner Kommentar ist mir noch aufgefallen, dass du häufig Array.at benutzt; wie sicher bist du dass das safe ist und nicht gelegentlich die ganze UI crasht? Uh, zumindest in Node20 wirft das nicht mehr, again what learned.

Wir nutzen in der GRUD nirgends das function-Keyword, sondern eigentlich immer Arrow-Functions; es wäre gut, wenn du das zumindest in den Closures noch änderst, in den export default-Fällen ist es egal.

[smnhgn]

Ich habe ien Review der UI gemacht und mir sind dabei ein paar Punkte aufgefallen:

1. Im entsperren-Button ist nur die Schrift klickbar. Außerhalb davon (aber innerhalb des Buttons) passiert nichts:

2. wählt man auf der linken Seite eine Linkspalte ohne Einträge wird rechts der Text "No linked entries available" angezeigt. Egal welche Sprache ausgewählt ist. (Übersetzung fehlt?)

3. In den Auswahlmöglichkeiten auf der rechten Seite sind nur die Checkboxen klickbar. Erwartung wäre Checkbox + Label klickbar.

4. Aktuell können sowohl Currency als auch Boolean-Spalten als Titel gewählt werden.
   mMn. machen diese Spaltentypen im Titel keinen Sinn und sollten gefiltert/ausgeblendet werden.
   (Preise im Titel machen UI kaputt und Boolean "Ja" oder "Nein" hat keinen Mehrwert ohne Kontext)

5. Titel wrapped nicht und wird bei zu vielen Spalten im Titel abgeschnitten.

6. Hover über Titel sollte mMn. beim Hovern die zugehörige Spalte zeigen (analog zu Hover über Spalten-Werte).

7. Beim Hovern über Werte einer verlinkten Taxonomy-Tabelle wird der Spaltenname nicht angezeigt:

[TimoGebendorfer]

@smnhgn

Habe alle Punkte außer Punkt 6. aus deinen Anmerkungen umgesetzt. Bzgl. dem Title rede ich dann erstmal noch mit Max wegen dem Identifier als default title und ob dann auch noch gleich wegen dem hover

[hermann-p]

Aktuell können sowohl Currency als auch Boolean-Spalten als Titel gewählt werden.
mMn. machen diese Spaltentypen im Titel keinen Sinn und sollten gefiltert/ausgeblendet werden.
(Preise im Titel machen UI kaputt und Boolean "Ja" oder "Nein" hat keinen Mehrwert ohne Kontext)

Für Boolean könnte man auch die allseits beliebte getDisplayValue-Funktion heranziehen, die entweder den displayName der Spalte oder einen leeren String liefert (wird auch in Identifiern häufig benutzt, "straßentauglich" ist so ein Fall). Currency als Titel ergibt keinen Sinn, weil das ja länderspezifisch ist. Um etwaitigen zukünftigen Umbauten vorzubeugen könnte man Spalten verbieten mit languageType === LanguageType.country.

[smnhgn]

1. Bei der Auswahl einer LinkColumn werden die verlinkten Rows doppelt geladen. (siehe Network-Tab):

bug-previewcenter-rows-duplicate-load.mp4

2. In der Auflistung der Spalten werden GroupColumn und die darin enthaltenen Columns separat gelistet. (Die Links der DetailColumns gehen ins Leere, weil in den Tabellen standardmäßig nur die GroupColumn angezeigt wird):

3. Bei zu wenig Platz auf der linken Seite ist kein horizontales scrollen möglich.

[TimoGebendorfer]

@smnhgn die Angemerkten Punkte von dir sind umgesetzt.

Lediglich Punkt 3 nicht (Bei zu wenig Platz auf der linken Seite ist kein horizontales scrollen möglich), da das vom Verhalten her so passt meint Julia. Wenn man den abgeschnitten Text sehen möchte kann man ja den Resizer dafür verwenden anstatt noch eine extra Scrollbar anzuzeigen

[hermann-p]

Link-Ansicht

Wenn ich eine Linkspalte auswähle und Refreshe bzw. direkt einem Deeplink zu einer Linkspalte mit vorhandenen Werten folge, dann ist die rechte Hälfte leer mit "Es sind keine verknüpften Einträge vorhanden.". Wenn ich woanders hin- und wieder zurückklicke werden die verlinkten Rows korrekt angezeigt.

Der "alle auswählen"-Checkmark ist buggy. Wenn ich auf das Checkmark klicke werden korrekt alle (de-) selektiert, die Funktionalität passt also. Wenn ich allerdings entweder manuell Einträge abwähle oder per Klick auf den restlichen Button (also z.B. auf den Text), dann ignoriert der Checkmark das. Vermutlich hat der Checkmark entweder internen State oder wird als uncontrolled-input genutzt.
Meine Erwartung wäre, dass der Checkmark selbst Mausevents ignoriert und diese nur vom darunterliegenden Button verarbeitet werden. Der Checkmarkstate von "Alle auswählen" sollte dann dadurch errechnet werden, ob wirklich gerade alle selektiert sind und als Wert eines controlled-input gesetzt werden (hierfür würds z.B. reichen, einfach zu zählen wieviele Links es insgesamt gibt und wieviele selektiert sind).

Attachment-Ansicht

Alle Attachments werden als Bilder gewertet und im Karussell als kaputte Vorschau angezeigt, falls sie keine Bilder sind (getestet mit .md, .wav, .html, .pdf).

[TimoGebendorfer]

@hermann-p der Reload-Bug ist gefixt.

Das "alle auswählen"-Checkmark Verhalten habe ich angepasst. Aber beibehalten dass man mit einem Klick auf das Checkmark alle an- bzw. abwählen kann. Das ist praktisch wenn man bei mehreren Links nur zwei miteinander verglichen möchte. Dann muss man nicht erst alle bis auf die zwei deselektieren.

Bzgl. Attachment-Ansicht bei "Nicht-Bildern" haben wir noch nicht definiert was wir in dem Fall anzeigen (sollten wir vll mal tun ;P)

Veraltet

[hermann-p]

Soweit ich das erkennen kann schauts ganz gut aus so.

[smnhgn]

Ein letzter Punkt ist mir noch aufgefallen, den wir klären/fixen sollten bevor wir den PR mergen:

Aktuell kann man in einem Union-Table Überprüfen auswählen, aber beim Öffnen der Preview kommt es zu einem Fehler:

D.h. die Preview sollte noch gezielt für Union-Table getestet/gefixt werden.

[TimoGebendorfer]

@hermann-p @smnhgn gerne nochmal kurz anschauen und Bescheid geben wenn ich mergen soll

[smnhgn]

Bis auf die Sachen die ich angemerkt habe läuft alles rund 👍

[smnhgn]

Hab mich nochmal durchgeklickt und für mich passts jetzt.

[zingmane]

@TimoGebendorfer
Bitte noch Konflikte auflösen, damit wir mergen können.

[TimoGebendorfer]

Welche Konflikt @zingmane? Der branch setzt auf dem aktuellsten Stand von master auf

[zingmane]

Hm, komisch. Vorhin hat mir github den merge wegen Konflikten verwehrt...
Jetzt bracuht es wieder einen approve 🥴
Könnt ihr code owner das bitte noch mal anschaun und dann mergen, @hermann-p @smnhgn