[webkit-reviews] review denied: [Bug 193260] Web Inspector: Event breakpoints: up/down keys don't always work : [Attachment 358657] Patch

[bugzilla-daemon at webkit.org]

Nikita Vasilyev <nvasilyev at apple.com> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 193260: Web Inspector: Event breakpoints: up/down keys don't always work
https://bugs.webkit.org/show_bug.cgi?id=193260

Attachment 358657: Patch

https://bugs.webkit.org/attachment.cgi?id=358657&action=review




--- Comment #3 from Nikita Vasilyev <nvasilyev at apple.com> ---
Comment on attachment 358657
  --> https://bugs.webkit.org/attachment.cgi?id=358657
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358657&action=review

> Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.js:243
> +	   if (!isNaN(oldSelectedIndex)) {
> +	       let sway = 0;
> +	       while (true) {
> +		   let itemBefore = oldCompletions[oldSelectedIndex - sway];
> +		   let itemAfter = oldCompletions[oldSelectedIndex + sway];
> +		   if (!itemBefore && !itemAfter) {
> +		       selectedIndex = 0;
> +		       break;
> +		   }
> +
> +		   let indexBefore = completions.indexOf(itemBefore);
> +		   if (indexBefore !== -1) {
> +		       selectedIndex = indexBefore;
> +		       break;
> +		   }
> +
> +		   let indexAfter = completions.indexOf(itemAfter);
> +		   if (indexAfter !== -1) {
> +		       selectedIndex = indexAfter;
> +		       break;
> +		   }
> +
> +		   ++sway;
> +	       }
> +	   }

I like that you're trying to find previously selected item. It just feels
right.

r- because I don't understand the use case for `sway > 0`. I don't see how it's
useful when typing a character or deleting a character. Please provide the
exact steps when it makes sense.