fixed-when searching while hovering over a place search result, highlights become visually stuck

[draunger]

description

problem before the fix:

• when we search the place by placing the cursor on the empty area initially ,
• then after writing the place, the search result comes and as the cursor is on the item
• it highlight that item on the map , so after changing the place by placing the cursor on the same position as before
• the item is remain highlighted or if we change the place without changing the position of the cursor the item
• which is hovered before is highlighted along with the new result
• here you can see in this video attached below:-

https://www.loom.com/share/3ff0436cf2854bed8f390df3710ad5f1?sid=97a6949d-10f5-46e5-8144-60158e730c18

as you can see in this video when we write "dek" then kish hospitality drive also highlighted along with dekalb...

approach:

• Basically i used a set data structure and push every id of highlighted item
• when we change the search label then the previous highlighted items get unhighlighted
• i used this in if-condition and iterate in set data structure so that all the previous highlighted item get unhighlighted
• here you can see in this video atttached below :

https://www.loom.com/share/a0eff1338e69448b8c16d2d020039528?sid=411b93af-545b-48e7-9e71-a0d10a9180f6

attached issue :

#10661

[tyrasd]

Can you please describe your thoughts behind this bugfix? What were the causes of the bug to occur in the first place and how do your changes address these?

Your code should further match the style of the rest of the codebase: for example we use camel-case instead of underscores in variable names, and whitespace should be consistently useed around =.- Please clean this up.

Finally, please use a descriptive title for your pull request and don't shout in ALL-CAPS. 🙏

[draunger]

@tyrasd i have describe my thoughts and also the describe the issue in detail , very sorry for ALL-CAPS 🙏🙏

pr updated

[tyrasd]

Ok, thanks for that.

The downside of your approach is that this "solution" it is merely addressing the symptom and not the cause of the underlying problem. Admittedly, this was not a trivial one to figure out, but here is what I noticed:

• There are event handlers set for both mouseover and mouseout, which should "in theory" already disable the highlighted state of the previously selected entity when the mouse enters a different element.
• For some reason, the mouseout handler is sometimes not called. There must be some underlying problem causing this event handler not to be triggered properly.
• One of the cases where the event handler is not called is when the DOM element is removed from the page (in items.exit().remove()). This makes sense: when there is no element, there are also no events. So, to fix this case, we need to manually call the mouseout handler when DOM elements are removed. Like this: items.exit().each(d => mouseout(undefined, d)).remove();
• This fixes some cases of the bug, but not all: when the mouse is on the first search result, the highlight can still get stuck. After a bit of debugging, I noticed that items.enter() had always 1 DOM element, even if the search result list had the same length as previously. This should not be the case, as d3 should never have to create new DOM elements when they already exist. So, something must be wrong with the data binding of the search results to the DOM.
• The culprit turned out to be a few lines further above:

            list.selectAll('.feature-list-item')
                .data([-1])
                .remove();

  This was added a long time ago with Adds support for ids and locations as search input #2056.
• This indeed messes up the binding of the first DOM element of the items selection to the first search result, as it is always removed whenever the search results list is updated.
• This explains why the mouseout event is not called for the first element of the search results list, when one types in the search field: As the element is always deleted and immediately recreated in the following items.enter() instruction, the mouse events don't have a chance to be properly triggered.
• I cannot see what useful things the ….data([-1]).remove() instruction would be meant to doing here: it's not related to the functionality of the PR where it was added, and it generally does not seem to make much sense. 🤷 My best guess is that it was accidentally included in the commit and overlooked in the PR review back then.
• Simply removing the culprit 3 lines of code seems to finally fully fix the original issue. Most importantly, this does not require adding any workaround and additional complexity to the code.