From 929cf2983cedfae24e696397d8d71e827f706a7f Mon Sep 17 00:00:00 2001 From: mattcasters Date: Sat, 8 Aug 2026 15:02:48 +0200 Subject: [PATCH] issue #7832 : re-enable data preview sorting and column metadata tooltips ShowRowsDialog can sort columns again while keeping full cell-value lookup working via preserved buffer indexes. Selected and hovered cells show name, type, length, precision, and origin tooltips. --- .../ShowRowsDialogCellSelectionTest.java | 40 +++++ .../hop/ui/core/dialog/ShowRowsDialog.java | 147 ++++++++++++++---- .../apache/hop/ui/core/widget/TableView.java | 27 +++- .../dialog/messages/messages_en_US.properties | 5 + .../dialog/ShowRowsDialogTooltipTest.java | 89 +++++++++++ 5 files changed, 276 insertions(+), 32 deletions(-) create mode 100644 ui/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogTooltipTest.java diff --git a/rcp/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogCellSelectionTest.java b/rcp/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogCellSelectionTest.java index 202bac6b54..b5c3b85ce5 100644 --- a/rcp/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogCellSelectionTest.java +++ b/rcp/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogCellSelectionTest.java @@ -88,6 +88,46 @@ void clickingACellDropsAReadOnlyEditorHoldingTheFullValue() { LONG_VALUE, onUi(editor::getText), "the editor must expose the full, untruncated cell value for copying"); + String tooltip = onUi(editor::getToolTipText); + assertNotNull(tooltip, "selected cell should show a column-metadata tooltip"); + assertTrue( + tooltip.contains("text"), + "cell tooltip should include the column name; was: " + tooltip); + }); + } + + @Test + void sortingKeepsFullValueLookupForTheSelectedCell() { + IRowMeta rowMeta = new RowMeta(); + rowMeta.addValueMeta(new ValueMetaString("text")); + List rows = new ArrayList<>(); + rows.add(new Object[] {"zebra-" + LONG_VALUE}); + rows.add(new Object[] {"alpha-" + LONG_VALUE}); + + withDialog( + parent -> + new ShowRowsDialog(parent, new Variables(), "Preview", "Output rows", rowMeta, rows) + .open(), + bot -> { + Table table = awaitTable(bot); + assertNotNull(table, "the ShowRowsDialog table should open"); + + // Sort by the data column (index 1; 0 is the row-number column). + onUi( + () -> { + table.getColumn(1).notifyListeners(SWT.Selection, new Event()); + return null; + }); + bot.sleep(100); + + // After ascending sort, "alpha-..." should be first. Click it and verify full value. + clickFirstDataCell(table); + Text editor = awaitCellEditor(bot, table); + assertNotNull(editor, "cell editor should open after sort"); + assertEquals( + "alpha-" + LONG_VALUE, + onUi(editor::getText), + "after sorting, the full buffer value for the clicked row must still be resolved"); }); } diff --git a/ui/src/main/java/org/apache/hop/ui/core/dialog/ShowRowsDialog.java b/ui/src/main/java/org/apache/hop/ui/core/dialog/ShowRowsDialog.java index b6dab857de..b62dd882e9 100644 --- a/ui/src/main/java/org/apache/hop/ui/core/dialog/ShowRowsDialog.java +++ b/ui/src/main/java/org/apache/hop/ui/core/dialog/ShowRowsDialog.java @@ -18,6 +18,7 @@ package org.apache.hop.ui.core.dialog; import java.util.List; +import java.util.Objects; import org.apache.commons.codec.binary.Hex; import org.apache.hop.core.Const; import org.apache.hop.core.config.HopConfig; @@ -53,6 +54,10 @@ *

Use this when the caller already has rows in memory and only needs to display them. For * transform preview (streaming, "get more rows", pause/stop, logging text), use {@link * PreviewRowsDialog} instead. + * + *

Column headers are sortable. Each table item stores its original buffer index via {@link + * TableItem#setData(Object)} so cell selection and full-value lookup still work after a sort + * ({@link TableView} preserves unkeyed item data when rebuilding rows). */ public final class ShowRowsDialog { @@ -155,7 +160,7 @@ private TableView buildTableView(int margin, Label messageLabel) { IValueMeta valueMeta = rowMeta.getValueMeta(i); columns[i] = new ColumnInfo(valueMeta.getName(), ColumnInfo.COLUMN_TYPE_TEXT, valueMeta.isNumeric()); - columns[i].setToolTip(valueMeta.toStringMeta()); + columns[i].setToolTip(formatColumnMetaTooltip(valueMeta)); columns[i].setValueMeta(valueMeta); columns[i].setImage(GuiResource.getInstance().getImage(valueMeta)); columns[i].setReadOnly(true); @@ -171,10 +176,9 @@ private TableView buildTableView(int margin, Label messageLabel) { null, PropsUi.getInstance()); view.setShowingBlueNullValues(true); - // Rows are kept in load order so a cell's visual position maps straight back to the buffer that - // holds its full value (see getFullCellString). Sorting rebuilds the table items from their - // truncated display text, breaking that mapping, so it is disabled here. - view.setSortable(false); + // Column sorting is enabled; each item stores its original buffer index so full-value lookup + // still works after the table is reordered (see resolveBufferIndex). + view.setSortable(true); FormData fdTable = new FormData(); fdTable.left = new FormAttachment(0, 0); @@ -194,18 +198,21 @@ private void populateRows() { } else { item = new TableItem(tableView.table, SWT.NONE); } - fillRow(item, rows.get(i)); + fillRow(item, rows.get(i), i); } if (!tableView.isDisposed()) { tableView.optWidth(true, 200); } } - private void fillRow(TableItem item, Object[] row) { + private void fillRow(TableItem item, Object[] row, int bufferIndex) { if (row == null) { return; } + // Unkeyed data: TableView preserves getData()/setData(Object) across column sorts. + item.setData(bufferIndex); + lineNr++; String rowNumber; try { @@ -264,7 +271,8 @@ private void close() { * TableView#formatCellValueForDisplay}). Clicking a cell drops a read-only text field on it (like * the inline editor of an editable grid) so its full value can be selected and copied in place; * double-clicking a cell shows its full, original content in a floating box (handy for long - * strings, JSON and multi-line values). + * strings, JSON and multi-line values). Hovering (or selecting) a cell shows column metadata in a + * tooltip: name, type, length, precision. */ private void setupCellSelection() { cellEditor = new TableEditor(tableView.table); @@ -280,12 +288,61 @@ private void setupCellSelection() { }); tableView.table.addListener( SWT.MouseDoubleClick, event -> showFullCellValue(new Point(event.x, event.y))); + // Dynamic cell tooltip with column metadata (name, type, length, precision, ...). + tableView.table.addListener( + SWT.MouseMove, + event -> { + CellRef ref = cellAt(new Point(event.x, event.y)); + String tip = + ref == null ? null : formatColumnMetaTooltip(rowMeta.getValueMeta(ref.dataColumn)); + if (!Objects.equals(tip, tableView.table.getToolTipText())) { + tableView.table.setToolTipText(tip); + } + }); + } + + /** + * Build a multi-line tooltip describing a column: name, type, and optional length / precision / + * origin. + */ + static String formatColumnMetaTooltip(IValueMeta valueMeta) { + if (valueMeta == null) { + return null; + } + StringBuilder tip = new StringBuilder(); + tip.append( + BaseMessages.getString( + PKG, "ShowRowsDialog.CellTooltip.Name", Const.NVL(valueMeta.getName(), ""))); + tip.append(Const.CR); + tip.append( + BaseMessages.getString( + PKG, "ShowRowsDialog.CellTooltip.Type", Const.NVL(valueMeta.getTypeDesc(), ""))); + if (valueMeta.getLength() > 0) { + tip.append(Const.CR); + tip.append( + BaseMessages.getString( + PKG, "ShowRowsDialog.CellTooltip.Length", Integer.toString(valueMeta.getLength()))); + } + if (valueMeta.getPrecision() > 0) { + tip.append(Const.CR); + tip.append( + BaseMessages.getString( + PKG, + "ShowRowsDialog.CellTooltip.Precision", + Integer.toString(valueMeta.getPrecision()))); + } + if (!Utils.isEmpty(valueMeta.getOrigin())) { + tip.append(Const.CR); + tip.append( + BaseMessages.getString(PKG, "ShowRowsDialog.CellTooltip.Origin", valueMeta.getOrigin())); + } + return tip.toString(); } private void showFullCellValue(Point point) { CellRef ref = cellAt(point); if (ref != null) { - expandCell(ref.bounds, ref.rowIndex, ref.columnIndex); + expandCell(ref.bounds, ref.bufferIndex, ref.tableColumn); } } @@ -299,11 +356,11 @@ private void openCellEditor(Point point) { if (ref == null) { return; } - String full = getFullCellString(ref.rowIndex, ref.columnIndex - 1); + String full = getFullCellString(ref.bufferIndex, ref.dataColumn); if (full == null) { return; } - TableItem item = tableView.table.getItem(ref.rowIndex); + TableItem item = tableView.table.getItem(ref.tableRowIndex); if (cellEditorText != null && !cellEditorText.isDisposed()) { cellEditorText.dispose(); @@ -312,6 +369,7 @@ private void openCellEditor(Point point) { final Text field = new Text(tableView.table, SWT.SINGLE | SWT.READ_ONLY); PropsUi.setLook(field); field.setText(full); + field.setToolTipText(formatColumnMetaTooltip(rowMeta.getValueMeta(ref.dataColumn))); cellEditorText = field; final long openedAt = System.currentTimeMillis(); @@ -330,25 +388,25 @@ private void openCellEditor(Point point) { // plain MouseDown; treat a click within the OS double-click time of it opening as that second // click too. Coordinates are captured so the box anchors to the same cell. final Rectangle cellBounds = ref.bounds; - final int rowIndex = ref.rowIndex; - final int columnIndex = ref.columnIndex; - field.addListener(SWT.MouseDoubleClick, e -> expandCell(cellBounds, rowIndex, columnIndex)); + final int bufferIndex = ref.bufferIndex; + final int tableColumn = ref.tableColumn; + field.addListener(SWT.MouseDoubleClick, e -> expandCell(cellBounds, bufferIndex, tableColumn)); field.addListener( SWT.MouseDown, e -> { if (System.currentTimeMillis() - openedAt <= tableView.getDisplay().getDoubleClickTime()) { - expandCell(cellBounds, rowIndex, columnIndex); + expandCell(cellBounds, bufferIndex, tableColumn); } }); - cellEditor.setEditor(field, item, columnIndex); + cellEditor.setEditor(field, item, tableColumn); field.setFocus(); field.selectAll(); } /** Expand the given cell's full value into the floating, selectable value box. */ - private void expandCell(Rectangle cellBounds, int rowIndex, int columnIndex) { + private void expandCell(Rectangle cellBounds, int bufferIndex, int tableColumn) { if (cellEditorText != null && !cellEditorText.isDisposed()) { cellEditorText.dispose(); } @@ -357,7 +415,7 @@ private void expandCell(Rectangle cellBounds, int rowIndex, int columnIndex) { if (valueOverlay != null && !valueOverlay.isDisposed()) { return; } - String full = getFullCellString(rowIndex, columnIndex - 1); + String full = getFullCellString(bufferIndex, tableColumn - 1); if (full != null) { showValueOverlay(cellBounds, full); } @@ -375,31 +433,66 @@ private CellRef cellAt(Point point) { if (item == null) { return null; } - int rowIndex = tableView.table.indexOf(item); - if (rowIndex < 0 || rowIndex >= rows.size()) { + int tableRowIndex = tableView.table.indexOf(item); + if (tableRowIndex < 0) { + return null; + } + int bufferIndex = resolveBufferIndex(item); + if (bufferIndex < 0 || bufferIndex >= rows.size()) { return null; } // Column 0 is the row-number column; data columns start at 1. Find the one under the pointer. for (int i = 1; i < tableView.table.getColumnCount(); i++) { Rectangle b = item.getBounds(i); if (b.contains(point)) { - return i - 1 < rowMeta.size() ? new CellRef(rowIndex, i, b) : null; + int dataColumn = i - 1; + return dataColumn < rowMeta.size() + ? new CellRef(bufferIndex, tableRowIndex, i, dataColumn, b) + : null; } } return null; } /** - * A located data cell: its row index into the buffer, its 1-based table column, and its bounds. + * Map a visual table item back to its original {@link #rows} index. Prefers the value stored at + * fill time via {@link TableItem#setData(Object)}; falls back to the 1-based line number in + * column 0, which also moves with the row when the table is sorted. + */ + private int resolveBufferIndex(TableItem item) { + Object data = item.getData(); + if (data instanceof Integer index) { + return index; + } + // Fallback: original line number text in the # column (1-based). + try { + String text = item.getText(0); + if (Utils.isEmpty(text)) { + return -1; + } + return Integer.parseInt(text.trim()) - 1; + } catch (NumberFormatException e) { + return -1; + } + } + + /** + * A located data cell: buffer index into {@link #rows}, visual table row, 1-based table column, + * 0-based data column, and cell bounds. */ private static final class CellRef { - private final int rowIndex; - private final int columnIndex; + private final int bufferIndex; + private final int tableRowIndex; + private final int tableColumn; + private final int dataColumn; private final Rectangle bounds; - private CellRef(int rowIndex, int columnIndex, Rectangle bounds) { - this.rowIndex = rowIndex; - this.columnIndex = columnIndex; + private CellRef( + int bufferIndex, int tableRowIndex, int tableColumn, int dataColumn, Rectangle bounds) { + this.bufferIndex = bufferIndex; + this.tableRowIndex = tableRowIndex; + this.tableColumn = tableColumn; + this.dataColumn = dataColumn; this.bounds = bounds; } } diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java b/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java index 47fbc5d709..3564dbd8fb 100644 --- a/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java +++ b/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java @@ -1712,13 +1712,25 @@ public void sortTable(int sortField, boolean sortingDescending) { final IRowMeta sourceRowMeta = buildTableSourceRowMeta(rowMeta, conversionRowMeta); List v = getTableItemsAsRows(items, sourceRowMeta); + // Keep TableItem#getData() across the rebuild so callers (e.g. ShowRowsDialog) can map a + // visual row back to an external buffer index after the user sorts a column. + final Object[] preservedData = new Object[items.length]; + for (int i = 0; i < items.length; i++) { + preservedData[i] = items[i].getData(); + } + final int[] sortIndex = new int[] {sortField + 2}; - // Sort the vector! - v.sort( - (r1, r2) -> { + // Sort indices so each row stays paired with its preserved widget data. + Integer[] order = new Integer[v.size()]; + for (int i = 0; i < order.length; i++) { + order[i] = i; + } + Arrays.sort( + order, + (i1, i2) -> { try { - return conversionRowMeta.compare(r1, r2, sortIndex); + return conversionRowMeta.compare(v.get(i1), v.get(i2), sortIndex); } catch (HopValueException e) { throw new HopRuntimeException("Error comparing rows", e); } @@ -1728,7 +1740,8 @@ public void sortTable(int sortField, boolean sortingDescending) { table.removeAll(); // Refill the table - for (Object[] r : v) { + for (int origIdx : order) { + Object[] r = v.get(origIdx); TableItem item = new TableItem(table, SWT.NONE); String colorName = (String) r[0]; @@ -1755,6 +1768,10 @@ public void sortTable(int sortField, boolean sortingDescending) { item.setText(j - 2, string); } } + + if (preservedData[origIdx] != null) { + item.setData(preservedData[origIdx]); + } } table.setSortColumn(table.getColumn(this.sortField)); table.setSortDirection(sortingDescending ? SWT.DOWN : SWT.UP); diff --git a/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties b/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties index 58fd859c8e..4589d50e52 100644 --- a/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties +++ b/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties @@ -299,6 +299,11 @@ PreviewRowsDialog.ShowLogging.Title=Logging text PreviewRowsDialog.Title=Examine preview data ShowRowsDialog.NoRows.Message=No rows to display. ShowRowsDialog.NrRows=({0} rows) +ShowRowsDialog.CellTooltip.Name=Name: {0} +ShowRowsDialog.CellTooltip.Type=Type: {0} +ShowRowsDialog.CellTooltip.Length=Length: {0} +ShowRowsDialog.CellTooltip.Precision=Precision: {0} +ShowRowsDialog.CellTooltip.Origin=Origin: {0} ProgressMonitorDialog.InitialTaskLabel=Apache Hop is handling a long running task ProgressMonitorDialog.Shell.Title=Progress... ProgressMonitorDialog.InitialSubTaskLabel=\ diff --git a/ui/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogTooltipTest.java b/ui/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogTooltipTest.java new file mode 100644 index 0000000000..c687e30d1a --- /dev/null +++ b/ui/src/test/java/org/apache/hop/ui/core/dialog/ShowRowsDialogTooltipTest.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.ui.core.dialog; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.hop.core.HopEnvironment; +import org.apache.hop.core.row.value.ValueMetaNumber; +import org.apache.hop.core.row.value.ValueMetaString; +import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Unit tests for {@link ShowRowsDialog#formatColumnMetaTooltip}: cell tooltips must expose column + * metadata (name, type, length, precision) as requested in issue #7832. + */ +@ExtendWith(RestoreHopEngineEnvironmentExtension.class) +class ShowRowsDialogTooltipTest { + + @BeforeAll + static void initHop() throws Exception { + HopEnvironment.init(); + } + + @Test + void nullValueMetaReturnsNull() { + assertNull(ShowRowsDialog.formatColumnMetaTooltip(null)); + } + + @Test + void stringFieldTooltipIncludesNameTypeAndLength() { + ValueMetaString meta = new ValueMetaString("customer_name"); + meta.setLength(100); + meta.setOrigin("Customers"); + + String tip = ShowRowsDialog.formatColumnMetaTooltip(meta); + assertNotNull(tip); + assertTrue(tip.contains("customer_name"), "tooltip should include the field name"); + assertTrue(tip.contains("String") || tip.contains("string"), "tooltip should include the type"); + assertTrue(tip.contains("100"), "tooltip should include the length"); + assertTrue(tip.contains("Customers"), "tooltip should include the origin when set"); + } + + @Test + void numberFieldTooltipIncludesPrecision() { + ValueMetaNumber meta = new ValueMetaNumber("amount"); + meta.setLength(12); + meta.setPrecision(2); + + String tip = ShowRowsDialog.formatColumnMetaTooltip(meta); + assertNotNull(tip); + assertTrue(tip.contains("amount"), "tooltip should include the field name"); + assertTrue(tip.contains("12"), "tooltip should include the length"); + assertTrue(tip.contains("2"), "tooltip should include the precision"); + } + + @Test + void omitsUnsetLengthAndPrecision() { + ValueMetaString meta = new ValueMetaString("notes"); + // length/precision left at defaults (-1) + + String tip = ShowRowsDialog.formatColumnMetaTooltip(meta); + assertNotNull(tip); + assertTrue(tip.contains("notes")); + // No dedicated length/precision lines when values are not positive + assertFalse(tip.toLowerCase().contains("length:"), tip); + assertFalse(tip.toLowerCase().contains("precision:"), tip); + } +}