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

Revert "Fix duplicate id="logout-form" in "combo" layout mode" #80

Merged

Conversation

ser-tec
Copy link
Contributor

@ser-tec ser-tec commented Dec 10, 2024

Reverts #79

Hi,

I must apologize as I didn’t properly test the pull request I recently submitted, and I sincerely regret the oversight. The issue lies in how the logout functionality is implemented, and the merged modification prevents the logout from working correctly. This needs to be reverted.

Additionally, the root problem should be addressed more thoroughly. The ideal solution would be to ensure the logout form is included only once in the layout to avoid duplication. Alternatively, we could use a single PHP variable to generate a unique ID that is then used consistently for both the link and the form. For example:

@php
    $logoutFormId = 'logout-form-'.uniqid();
@endphp

<div class="dropdown-divider"></div>
<a class="dropdown-item"
   href="#" onclick="event.preventDefault(); document.getElementById('{{ $logoutFormId }}').submit();">
    <i class="fa fa-fw fa-power-off text-red"></i>
    {{ __('tablar::tablar.log_out') }}
</a>

<form id="{{ $logoutFormId }}" action="{{ $logout_url }}" method="POST" style="display: none;">
    @if(config('tablar.logout_method'))
        {{ method_field(config('tablar.logout_method')) }}
    @endif
    {{ csrf_field() }}
</form>

Once again, I apologize for the inconvenience caused.

Best regards,
Giulio

@takielias
Copy link
Owner

@ser-tec That was my fault for not checking thoroughly before merging that PR. Anyway, thanks for the fix.

@takielias takielias merged commit 3210338 into takielias:master Dec 11, 2024
1 check passed
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