michalsn / codeigniter-htmx

HTMX helper library for CodeIgniter 4 framework
https://michalsn.github.io/codeigniter-htmx/
MIT License
75 stars 15 forks source link

feat: Improve redirects #79

Open neznaika0 opened 2 weeks ago

neznaika0 commented 2 weeks ago

I continue to work with HTMX. Should I redefine RedirectResponse(back(), route(), to()) and update redirect()?

$responseHTMX = single_service('redirectresponse')
        ->hxRedirect((string) previous_url(true))
        ->with(...$flashes)
        ;

$responseCI = redirect()->back()->with(...$flashes);

When the request came as HTMX, the redirect will have the initial headers. By making a regular redirect, the response will remain as for HTMX. Therefore, the second entry does not make sense. Now we need to duplicate the code for different cases (HTMX or a regular request). Would it be better to combine these conditions inside the new functions?

neznaika0 commented 2 weeks ago

Also, I have not found a function for the relative path. It would be great to simplify hxLocation() when passing the full path:

$responseHTMX = single_service('redirectresponse')->hxLocation((string) previous_url(true));
public function hxLocation(...): RedirectResponse {
    // $path = 'http://example.local/admin/users?searchQuery=John&sort=ASC';
    /**
     * @var URI $uri
     */
    $uri  = single_service('uri', $path)->withScheme('')->setHost('');
    $data = ['path' => '/' . ltrim((string) $uri, '/')];
    // to '/admin/users?searchQuery=John&sort=ASC'
}
michalsn commented 2 weeks ago

The redirect() and hxRedirect() behave differently and set different headers.

If we send htmx request it doesn't mean automatically we will want to use hxRedirect() and vice versa.

I want to keep the current methods intact as they are the equivalent of the pure htmx implementation. However, I'm open to introducing some sort of helper function that would serve for auto-recognizing the type of request and then serving different responses.

function hxRedirect()
{
 // it htmx then this
 // and if normal request then that
}
neznaika0 commented 2 weeks ago

Yes, I mentioned the headers. This is the reason for the change. It doesn't make sense to have a 30x redirect if the request comes from HTMX. It only stealthily replace content from new route. And it adds complexity if 30x is supposed to reload the page.

Example, from the list of users with the "user-list" fragment, click the Ban/Unban button. With HTMX, you need hxRedirect() / hxLocation() and return fragments, but without it (click on the link <a>) you need 30x redirects with the full page. Otherwise (now), after the 30x redirect, the page sends a fragment of the "user-list".

Therefore, duplicate conditions arise.

        if ($this->request->isHtmx()) {
            return $responseHTMX->with(...$flashError);
        }

        return redirect()->back()->with(...$flashError);

Changing the function would be a good solution to use a single code. hxRedirect() may help, but the code for the methods back()... also needs to be improved and hxRedirect() loses its meaning.

PS: Sorry for the machine translation, if not everything is clearly described

michalsn commented 2 weeks ago

You can name this helper function autoRedirect() or something similar - this can be discussed as we go.

Unfortunately, I will not approve any changes to the existing methods, because we need to be able to decide whether we use htmx response or not. This decision cannot blindly rely on the HX-Request header by default.

neznaika0 commented 2 weeks ago

I heard you. Maybe someday there will be users with a similar problem and we will come back to this. It's easier for me to modify my project

neznaika0 commented 2 weeks ago

https://github.com/michalsn/codeigniter-htmx/issues/79#issuecomment-2410660298 Real improve?

michalsn commented 2 weeks ago

#79 (comment) Real improve?

Yes, but I would implement it a bit differently:

public function hxLocation(...): RedirectResponse {

    if (str_starts_with($path, 'http')) {
        $path = (string) service('uri', $path, false)->withScheme('')->setHost('');
    }

    $data = ['path' => '/' . ltrim($path, '/')];

    // ...
}

Plus this would need a mention in the docs.

neznaika0 commented 2 weeks ago

There may be a problem with http verification if the URL is "http/users/123"

michalsn commented 2 weeks ago

Good point, you can change it to http:// and https://

neznaika0 commented 2 weeks ago

I won't be able to create a PR for the next few weeks.

I would not use a shared service, but get a new. There may be a conflict when the URI is used outside the redirect