Skip to content

Commit ee8cb4c

Browse files
DiegoCardosoclaude
andcommitted
chore(combo-box): trim review-narrative comments in focusSelectedItem
Drop comments that explain choices made during the PR rather than non-obvious WHY of the resulting code. Historical context (why the microtask poll, why the connector holds the flag, full close-time reset rationale) moved to PR #9194. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 720458f commit ee8cb4c

2 files changed

Lines changed: 10 additions & 25 deletions

File tree

  • vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main

vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -421,11 +421,8 @@ public void setOverlayWidth(float width, Unit unit) {
421421
*/
422422
public void setFocusSelectedItem(boolean focusSelectedItem) {
423423
this.focusSelectedItem = focusSelectedItem;
424-
// The connector's setFocusSelectedItem becomes available only once
425-
// initLazy has run. Both this call and initLazy are queued via
426-
// executeJs, but on the first attach there's no guarantee initLazy
427-
// runs first. Poll via microtask until $connector is ready instead
428-
// of relying on runBeforeClientResponse ordering.
424+
// $connector is registered during initLazy. Poll via microtask
425+
// until it's available so attach order isn't load-bearing.
429426
runBeforeClientResponse(ui -> getElement().executeJs(
430427
"const apply = (val) => {" + " if (this.$connector) {"
431428
+ " this.$connector.setFocusSelectedItem(val);"

vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/resources/META-INF/resources/frontend/comboBoxConnector.js

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,7 @@ window.Vaadin.Flow.comboBoxConnector.initLazy = (comboBox) => {
334334
// Prevent setting the custom value as the 'value'-prop automatically
335335
comboBox.addEventListener('custom-value-set', (e) => e.preventDefault());
336336

337-
// Feature flag for the focus-selected-item behavior. Set from the Java side
338-
// via `comboBox.$connector.setFocusSelectedItem(value)` so it doesn't need
339-
// to live as an element property the web component would otherwise ignore.
337+
// Set from the Java side via `comboBox.$connector.setFocusSelectedItem(value)`.
340338
let focusSelectedItemEnabled = false;
341339
comboBox.$connector.setFocusSelectedItem = (value) => {
342340
focusSelectedItemEnabled = !!value;
@@ -421,23 +419,13 @@ window.Vaadin.Flow.comboBoxConnector.initLazy = (comboBox) => {
421419

422420
comboBox.addEventListener('vaadin-combo-box-dropdown-opened', resolveFocusSelectedItem);
423421

424-
// Arm a DataCommunicator reset when the dropdown closes with
425-
// focusSelectedItem enabled. On the next open, the feature scrolls to the
426-
// selected item's position — the same viewport range the server last
427-
// sent — and the server would no-op the fetch, leaving pending callbacks
428-
// unresolved. Forcing a reset on the next setViewportRange RPC ensures
429-
// the server re-sends data for the requested range.
430-
//
431-
// The reset also re-keys items via the data communicator's normal
432-
// passivate-and-unregister flow — items outside the post-reset active
433-
// range have their KeyMapper entries removed and get fresh keys when
434-
// re-fetched. Cached pages in the connector and the data-provider
435-
// controller would then hold items with stale keys that no longer match
436-
// the freshly-keyed selectedItem, breaking selection highlighting (and
437-
// leaving them as targets for the focusSelectedItem RPC's scrollToIndex
438-
// when it sees a real item at the resolved index but with a stale key).
439-
// Wipe all client-side cache state on close so the next open populates
440-
// from the post-reset server data with current keys.
422+
// On close with focusSelectedItem on, arm a DataCommunicator reset and
423+
// wipe all client-side cache. The next open scrolls to the same viewport
424+
// range the server last sent, which would otherwise be a no-op fetch and
425+
// leave pending callbacks unresolved. The reset also re-keys items
426+
// outside the post-reset active range; cached pages holding the old keys
427+
// would no longer match the freshly-keyed selectedItem, breaking the
428+
// resolveSelectedItemIndex → scrollToIndex flow.
441429
comboBox.addEventListener('opened-changed', (e) => {
442430
if (e.detail.value === false && focusSelectedItemEnabled && comboBox.selectedItem) {
443431
serverFacade.needsDataCommunicatorReset();

0 commit comments

Comments
 (0)