Added ⌥E (prEset) for playground - preset button
Ideally this would be typed as Button instead of any, but I can see we're trying to get around the protected buttonRef property.
I think it's hacky, but everything seems hacky. Maybe we can type it as Button, but keep the any cast in line 245 and add a comment about it?
why ⌥X? (just don't see the thread that ties X to presets or partitions). Or
what about something like ⌥R (pReset) or ⌥C (Config)?
After thinking about this more, I think reaching into the Button component protected refs is a bad idea.
The ideal solution would be to rewrite the ConfigGeneratorSelect to be a controlled component (https://reactjs.org/docs/forms.html#controlled-components) and then have the onShortcut handler just change state directly. You might want to take a look at this section of the Blueprint docs (https://blueprintjs.com/docs/#select/select-component.controlled-usage)
Short of that, it might be better to take the DOM-based approach in https://dagster.phacility.com/source/dagster/browse/master/js_modules/dagit/src/PipelineJumpComponents.tsx.
But maybe @bengotow can comment...
Hey! Just wanted to chime in — I think that the final approach here (using the DOM APIs to "click" the select when the shortcut is activated) is probably the best. I ran into this with the PipelineJumpBar as well. Ideally, the select Component would expose public methods (class components can have instance methods that you can call to imperatively, eg myCustomComponentRef.zoomToCenter()) OR allow for controlled usage. Blueprint's Select component is more or less a black box, and activating it via the DOM is the least-likely-to-break hack, since it should fundamentally always respond to a click event.
I actually wonder if it might make sense to bake this behavior into the <ShortcutHandler /> itself. Maybe if no onShortcut method is provided, it could get it's own ReactDOM node, find the closest button child and click it as a default behavior? Just a thought - might not be worth it until we uncover one or two more use cases!
Hey @bengotow! Thanks for the context. Ya I think it would be a lot neater to go with your proposal. But I'd still prefer to land this workaround for now because this feature doesn't look like a top priority. I will create an issue so we have this context in backlog. Lemme know if that works :)