-
Notifications
You must be signed in to change notification settings - Fork 0
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
Introduce stepper dialog #934
base: development
Are you sure you want to change the base?
Conversation
Quality Gate failedFailed conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Easy to understand, Works and looks nice! Well done 👍 (I have some minor questions as always 🧠 though)
/** | ||
* <b><class short description - 1 Line!></b> | ||
* | ||
* <p><More detailed description - When to use, what it solves, etc.></p> | ||
* | ||
* @since <version tag> | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing JD.
if (steps.isEmpty()) { | ||
throw new IllegalArgumentException("Steps cannot be empty"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to allow a list of steps with only one steps here? 🤔
|
||
private final List<String> steps; | ||
|
||
private StepperDisplay(StepperDialog stepperDialog, List<String> stepNames) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason, why the stepNames are provided as strings? I would have expected, that the stepnames are extracted from the list of steps contained in the provided stepper dialog 🤔
# Support for editing Flow views with Copilot | ||
com.vaadin.experimental.copilotFlow=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's happening here?
import { UserConfigFn } from 'vite'; | ||
import { overrideVaadinConfig } from './vite.generated'; | ||
|
||
const customConfig: UserConfigFn = (env) => ({ | ||
// Here you can add custom Vite parameters | ||
// https://vitejs.dev/config/ | ||
}); | ||
|
||
export default overrideVaadinConfig(customConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could think about including the dev.bundle, prod.bundle and vite.config.ts file in the gitignore file, since they have to be rebuilt during development anyway 🤔
A stepper dialog (also known as wizard), contains at least two steps that are displayed consecutively to the user and ask them for input. They are usually applied when the user input is more complex, is conditionally chained, or can be separated contextually into logical steps.
This contribution introduces a stepper dialog component, that is easy to build and extend, and simplifies the stepper dialog creation.