Skip to content
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

Adds support for getting progress on individual steps #105

Closed
wants to merge 1 commit into from

Conversation

jonerickson
Copy link

The following PR adds a new property to the step classes that allows a user to determine the progress through the wizard. The property is set during the boot up procedure of the StepAware trait. I've added tests to the current suite as well.

I am looking for suggestions if this is the best place to put the logic - or should it be in the base Step component class? Also, should we add a method on the form class to determine progress overall using the currentStepName property? Thanks for the feedback!

@jonerickson jonerickson marked this pull request as draft January 11, 2024 22:31
@jonerickson jonerickson marked this pull request as ready for review January 20, 2024 02:48
@jonerickson
Copy link
Author

Any chance we can get this PR reviewed? Thanks!

Copy link
Member

@freekmurze freekmurze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this!

$this->getProgress();
}

public function getProgress(): float
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of calculating the percentage of the progress, I would just let the component be aware of the stepnumber and total number of steps.

Formatting this info as a percentage is then a task for the user of the package, and not the package itself.

@jonerickson
Copy link
Author

Closing as it can be done outside the package inside the Form component. Thanks for the help.

@jonerickson jonerickson closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants