sdedic commented on code in PR #7453:
URL: https://github.com/apache/netbeans/pull/7453#discussion_r1632707912
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/Steps.java:
##########
@@ -98,11 +100,82 @@ public CompletableFuture<Object> executeMultistep(Step
firstStep, Lookup lookup)
if (DialogDescriptor.OK_OPTION == dd.notify(ci)) {
future.complete(multistep.getResult());
} else {
- future.complete(null);
+ future.cancel(true);
}
});
return future;
}
+
+ /**
+ * Provider class that supplies the next {@link Step} for navigation from
the current {@link Step}.
+ *
+ */
+ public static class NextStepProvider {
Review Comment:
nitpick: final
Other thoughts -- I don't know where this Provider is leading to. The
"ultimate" goal may be just an utility usabe here, or something eventually
reusable.
In the code, it is used to specify a (complete) sequence of panels; the
panel instances are created at one place, in advance ad fed to the Builder.
That's OK, if no variability is ever expected (everything created upfront), but
in SuggestedItemStep, the current step's getValue() is used.
I would suggest to *defer* the instance creation to the time the user is
actually advancing to the next step: provide a Factory for the step instance,
rather than step instance itself. I'd suggest to reshape the Provider to have
a `Function<Step, Step>` instead of pre-created Step instance: this way the
logic can be moved to `AddNewAsset` that has the overall control over the
multi-step input. It can provide
```
builder.stepForClass(CompartmentStep.class, (s) -> new
SuggestedStep(s.getValue()))
```
It's also a question if the Provider should work based on currenct step's
Class, or just based on step's position - currently no alternate path selection
is used.
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/Steps.java:
##########
@@ -358,11 +427,14 @@ public NotifyDescriptor createInput() {
@Override
public Step getNext() {
- Step next = context.getNextStep();
- if (next != null) {
- next.prepare(getValue(), lookup);
- }
- return next;
+ NextStepProvider nsProvider =
lookup.lookup(NextStepProvider.class);
Review Comment:
Minor: Step could be an `abstract` class with this as the default impl of
`getNext()`. Or `Step` interace can implement this in its default method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
