Repository navigation
feat: Added search box to dynamic properties tab, added script search box support - #412
Conversation
BrandonChanSkybox
left a comment
BrandonChanSkybox
left a comment
There was a problem hiding this comment.
Approved with questions for understanding
|
|
||
| eventHandlersByName.set(statisticDataProviderName, eventHandler); | ||
| }); | ||
| statisticDataProviders.registerWindowListener(window); |
There was a problem hiding this comment.
Just leaving a question to make sure I understand correctly.
here you're using statisticDataProviders but in the previous version of the code it uses statsProvider which is an element in statisticDataProviders. Is statisticDataProviders no longer an array? Otherwise would we need to iterate through this still to run this method on all statsProviders still?
There was a problem hiding this comment.
Yeah before it was an array but both calling places only ever used a single prop with a single provider. So now this changed code more cleanly just extracts the single provider using the types above and uses it directly. The confusing part was that the the whole object (with all properties) before was called statisticDataProviders whereas now that name is used for the actual provider object itself within that.
The whole change was required because before it would have iterated on the new searchBox property (And it was also weirdly abstracted).
There was a problem hiding this comment.
It's a bit confusing but the new code is cleaner and hopefully makes more sense!
Added search box to dynamic properties by default:
Added tech to script driven diagnostics so that they can opt into a search box as well