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

Beautify Livewire Stack #326

Merged
merged 10 commits into from
Oct 29, 2023
Merged

Beautify Livewire Stack #326

merged 10 commits into from
Oct 29, 2023

Conversation

taylorotwell
Copy link
Member

Refactoring PR to clean up some code duplication in the Livewire stack as well as extract the complicated login logic into a Livewire Form. Also adds comments on class based stack.

Comment on lines +5 to +6
$logout = function (Logout $logout) {
$logout();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$logout = function (Logout $logout) {
$logout();
$logout = function (Logout $logoutAction) {
$logoutAction();

In my opinion it is a bad idea to have the same name for the callback and action, because I thought the Closure is calling itself.

Comment on lines +27 to +28
$logout = function (Logout $logout) {
$logout();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$logout = function (Logout $logout) {
$logout();
$logout = function (Logout $logoutAction) {
$logoutAction();

@taylorotwell taylorotwell merged commit c348b02 into 1.x Oct 29, 2023
16 checks passed
@taylorotwell taylorotwell deleted the beautify-livewire branch October 29, 2023 16:19
lancechentw added a commit to lancechentw/breeze that referenced this pull request Feb 20, 2024
The property names were not changed correspondingly to the refactors
from laravel#326.
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