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

Allow customizing field to use with login procedure #1318

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

al3nzy6
Copy link

@al3nzy6 al3nzy6 commented Nov 5, 2024

Question Answer
Pull request type ENHANCEMENT
License MIT

What's in this PR?

I don't want to use Email as field to login, I want to customize it, so I have add to adminlte config username_field in this package in view login.blade.php also change the field of username. Before it was only email now it has been able to customize.

Developer must add new function in LoginController:

protected function username() {
    return config('adminlte.username_field');
}

Checklist

  • I tested these changes.

@dfsmania dfsmania changed the title Patch 1 Allow customizing field to use with login procedure Nov 6, 2024
Comment on lines +30 to +31
<input type="{{ $username }}" name="{{ $username }}" class="form-control @error($username) is-invalid @enderror"
value="{{ old($username) }}" placeholder="{{ __('adminlte::adminlte.username') }}" autofocus>
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes introduces some bugs, first only a set of possible values are accepted by the input type attribute, so it can not be anything, note $username variable may hold any string name. Second, the translation entry __('adminlte::adminlte.username') does not currently exists.

@dfsmania
Copy link
Collaborator

dfsmania commented Nov 6, 2024

Hi @al3nzy6 , the idea is nice however we need to take into consideration other things:

  1. When this package was created, there was only one authentication scaffolding around Laravel, and it was Laravel/UI, so the package was intended to provide an easy way to integrate with it.

  2. Now there are more advanced authentication scaffoldings around, like Breeze and Jetstream which this package does not support by default. So, to not overcomplicate maintenance of the package, we just opted to allow publishing all the package resources and let the final user customize anything for their integration with any of these new auth scaffolding.

Now, I assume you have only tested these changes with legacy laravel/ui authentication scaffolding right? If that's the case I will try to look into this when I get some time, maybe we can approach to a better solution...

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