-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Quick fixes for the existing Executable / Despatcher UI #109
Comments
This relates to #681, where I suggest I slight rejig of the Executable/ExecutableNode setup. |
This tick box for replacing ExecutableUI (which doesn't exist, I assume it means ExecuteUI) with a DespatcherUI seems to be conflicting with a todo in StandardNodeUI, which wants to move some code to "a custom UI for the ExecutableNode". Almost all of what would be DespatcherUI would need to end up in ExecutableNodeUI. Shall I just rename ExecuteUI to ExecutableNodeUI, then add a new DespatcherUI for the despatcherPlug part of it? |
Or do you think the Execute button (and menu items) should be provided by DespatcherUI? If that's the case, should I be defining a class called ExecutableNodeUI to hold the button on the main tab (as it is now) but put such a class in DespatcherUI? Or should DespatcherUI define something else (not a NodeUI class) that adds the button to the Despatcher tab instead? |
Also, is ExecutableRenderUI intentionally hiding the requirement nodule or should that be fixed? |
Yep, let's rename ExecutableUI->ExecutableNodeUI. I think the menu items should probably be provided by DespatcherUI, and quite possibly the Execute button too. Currently the Execute button just calls execute() directly on the node, without using a Despatcher of any kind. We need to decide whether or not we would like to keep this functionality, or whether execution should always go through a Despatcher. I think I'm inclined to the latter, so that |
I think I probably just hid the requirements nodule in ExecutableRenderUI because we don't have despatchers integrated yet, and it was confusing to expose something that doesn't actually do anything yet. We should expose it again when it works... |
So... ExecuteUI -> ExecutableNodeUI, but the ExecuteButton (which is the current need for an ExecutableNodeUI class and makes up the bulk of the code in ExecuteUI) goes to DespatcherUI. Is that right? I guess it's all just commit history semantics then, so I might just keep my current (local) mess which moves ExecuteUI -> DespatcherUI, then adds ExecutableNodeUI as a new file, and steals some metadata lines from DespatcherUI. Does DespatcherUI add the ExecuteButton to the Despatcher tab then? Users might not like that it's hidden away... or maybe they'll get used to tab toggling to access all the other plugs. Or can the DespatcherUI add headers which go above the tabs altogether so the ExecuteButton is always exposed? |
The history juggling you mentioned sounds fine to me - it seems like git will always decide what was moved based on similarity of file contents rather than anything else anyway. I'd prefer the Execute/Despatch button to be on the main page rather than in the Despatcher tab as well. I think I'd want to be hitting that quite often, whereas I think the intention of the Despatcher tab was a place to set despatcher specific things like farm groups etc, which I would expect to be accessed less often. I guess we'll get a better feel for that sort of thing as we start to use Despatchers properly. You can move the UI for any plug into the header above the tabs by setting the |
The text was updated successfully, but these errors were encountered: