Commit 81ef43f
committed
refactor: fire select/preview events only for final segment during path restoration
Finder.select(FinderPath) previously fired SelectHandler and PreviewHandler
events for every intermediate segment. Now events are suppressed during the
sequential walk and fired once at the end for the deepest resolved item.
Also adds SelectHandler notification to ArrowLeft keyboard navigation, which
previously only fired PreviewHandler when re-focusing an already-selected item
in the previous column.1 parent 90d922c commit 81ef43f
2 files changed
Lines changed: 37 additions & 6 deletions
File tree
- extensions/finder/src/main/java/org/patternfly/extension/finder
Lines changed: 31 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -217,9 +217,10 @@ public Promise<ResolvedFinderPath> select(String path) { | |||
| 217 | 217 | * already be present in this finder. Each item selection may trigger creation and async loading of the next column (via | |
| 218 | 218 | * {@link FinderItem#nextColumn(java.util.function.Supplier)}). | |
| 219 | 219 | * <p> | |
| 220 | - * Select and preview events are fired for every item in the path. If a segment cannot be resolved (column not found, item | ||
| 221 | - * not found after loading), the promise resolves with a partial {@link ResolvedFinderPath} containing only the successfully | ||
| 222 | - * selected segments. | ||
| 220 | + * Select and preview events are fired only for the deepest successfully resolved item, not for intermediate segments. If a | ||
| 221 | + * segment cannot be resolved (column not found, item not found after loading), the promise resolves with a partial | ||
| 222 | + * {@link ResolvedFinderPath} containing only the successfully selected segments, and events are fired for the last resolved | ||
| 223 | + * item. | ||
| 223 | 224 | * | |
| 224 | 225 | * @param path the finder path to select | |
| 225 | 226 | * @return a {@link Promise} that resolves with the {@link ResolvedFinderPath} of successfully selected items | |
@@ -234,7 +235,10 @@ public Promise<ResolvedFinderPath> select(FinderPath path) { | |||
| 234 | 235 | tasks.add(ctx -> selectSegment(segment.columnId, segment.itemId, ctx)); | |
| 235 | 236 | } | |
| 236 | 237 | return sequential(context, tasks) | |
| 237 | - .then(ctx -> Promise.resolve(path())); | ||
| 238 | + .then(ctx -> { | ||
| 239 | + fireSelectAndPreview(); | ||
| 240 | + return Promise.resolve(path()); | ||
| 241 | + }); | ||
| 238 | 242 | } | |
| 239 | 243 | ||
| 240 | 244 | @Override | |
@@ -297,6 +301,24 @@ private void internalAdd(FinderColumn column) { | |||
| 297 | 301 | aur.added(column); | |
| 298 | 302 | } | |
| 299 | 303 | ||
| 304 | + private void fireSelectAndPreview() { | ||
| 305 | + FinderColumn lastColumn = null; | ||
| 306 | + FinderItem lastItem = null; | ||
| 307 | + for (FinderColumn column : items.values()) { | ||
| 308 | + FinderItem selected = column.selectedItem(); | ||
| 309 | + if (selected != null) { | ||
| 310 | + lastColumn = column; | ||
| 311 | + lastItem = selected; | ||
| 312 | + } else { | ||
| 313 | + break; | ||
| 314 | + } | ||
| 315 | + } | ||
| 316 | + if (lastColumn != null && lastItem != null) { | ||
| 317 | + lastColumn.fireSelect(lastItem); | ||
| 318 | + lastItem.previewItem(this, lastColumn, lastItem); | ||
| 319 | + } | ||
| 320 | + } | ||
| 321 | + | ||
| 300 | 322 | private Promise<FlowContext> selectSegment(String columnId, String itemId, FlowContext context) { | |
| 301 | 323 | if (context.get(STOP_SELECT_KEY) != null) { | |
| 302 | 324 | return context.resolve(); | |
@@ -317,12 +339,11 @@ private Promise<FlowContext> selectSegment(String columnId, String itemId, FlowC | |||
| 317 | 339 | context.set(STOP_SELECT_KEY, true); | |
| 318 | 340 | return context.resolve(); | |
| 319 | 341 | } | |
| 320 | - column.select(item); | ||
| 342 | + column.select(item, true, false); | ||
| 321 | 343 | FinderColumn nextColumn = item.supplyNextColumn(); | |
| 322 | 344 | if (nextColumn != null) { | |
| 323 | 345 | internalAdd(nextColumn); | |
| 324 | 346 | } | |
| 325 | - item.previewItem(this, column, item); | ||
| 326 | 347 | return context.resolve(); | |
| 327 | 348 | }); | |
| 328 | 349 | } | |
@@ -396,6 +417,10 @@ private void handleKeydown(KeyboardEvent event) { | |||
| 396 | 417 | scrollIntoView(previousColumn); | |
| 397 | 418 | FinderItem previousItem = previousColumn.item(targetItem.dataset.get(Dataset.identifier)); | |
| 398 | 419 | if (previousItem != null) { | |
| 420 | + // Don't use handleClick() here — it would call Finder.select(column) | ||
| 421 | + // which removes all columns after this one, collapsing the navigation tree. | ||
| 422 | + // Instead, fire select and preview events directly for the already-selected item. | ||
| 423 | + previousColumn.fireSelect(previousItem); | ||
| 399 | 424 | previousItem.previewItem(this, previousColumn, previousItem); | |
| 400 | 425 | } | |
| 401 | 426 | } | |
Lines changed: 6 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -476,6 +476,12 @@ public void select(FinderItem item, boolean selected, boolean fireEvent) { | |||
| 476 | 476 | ||
| 477 | 477 | // ------------------------------------------------------ internal | |
| 478 | 478 | ||
| 479 | + void fireSelect(FinderItem item) { | ||
| 480 | + if (item != null) { | ||
| 481 | + selectHandler.forEach(sh -> sh.onSelect(new Event(""), item, true)); | ||
| 482 | + } | ||
| 483 | + } | ||
| 484 | + | ||
| 479 | 485 | FinderItem findItem(String identifier) { | |
| 480 | 486 | return items.get(identifier); | |
| 481 | 487 | } | |
0 commit comments