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

feat: Add option to disable previous URL storage #82

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

neznaika0
Copy link
Contributor

Closes #81
Copied isWeb() - it private method
The test may be inaccurate, correct it

@michalsn
Copy link
Owner

michalsn commented Nov 5, 2024

I have second thoughts on this topic. When working with htmx, we may often want the page loaded by the htmx request to be stored as the previous URL.

My point is that in some cases this can be helpful, but for others, it will be a problem... so IMO we should make this option configurable.

Maybe $storePreviousURL option in the config file, with the default value set to true to preserve BC?

@neznaika0
Copy link
Contributor Author

neznaika0 commented Nov 5, 2024

An example of specific cases? Yes, the setup is not a problem
What about the AJAX option? It does not save by default. If the HTMX is configured with AJAX headers, the option has no effect.

@michalsn
Copy link
Owner

michalsn commented Nov 5, 2024

An example of specific cases?

An example would be using htmx in the navigation to simply navigate to another page. In that case, we won't be storing the previous URL in the session at all.

When AJAX headers are used, it is the user's choice. We will not override the default behavior of the framework.

@neznaika0
Copy link
Contributor Author

No 😄 . An example where the previous URL is needed.
In the userform > formhandler > userform chain, sometimes the previous URL may be formhandler.
I want to avoid this.

Plus, there are questions about the framework when working with browser tabs... there, too, the URL is overwritten.

I'll update the PR tomorrow. I would be grateful if we discuss it in more detail.

@michalsn
Copy link
Owner

michalsn commented Nov 5, 2024

It is not important where it will be used, only whether it will return correct data. If you disable updating the previous URL for htmx requests, then it may have an incorrect value.

Plus, there are questions about the framework when working with browser tabs... there, too, the URL is overwritten.

Yes, the same way as for traditional requests - we won't change that.

@michalsn michalsn changed the title fix: Disable save previous URL feat: Add option to disable previous URL storage Nov 6, 2024
docs/configuration.md Outdated Show resolved Hide resolved
src/CodeIgniter.php Outdated Show resolved Hide resolved
src/CodeIgniter.php Outdated Show resolved Hide resolved
src/Config/Htmx.php Outdated Show resolved Hide resolved
tests/CodeIgniterTest.php Outdated Show resolved Hide resolved
@neznaika0 neznaika0 force-pushed the fix-save-previous-page branch from 777ca8c to 573a4e7 Compare November 6, 2024 09:23
@neznaika0 neznaika0 requested a review from michalsn November 6, 2024 09:25
@neznaika0
Copy link
Contributor Author

You have almost completely corrected the PR. Fix it faster for you

Copy link
Owner

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

You did 90% of the work here. Thank you.

@michalsn michalsn merged commit 221fc70 into michalsn:develop Nov 6, 2024
14 checks passed
@neznaika0 neznaika0 deleted the fix-save-previous-page branch November 6, 2024 15:13
@neznaika0
Copy link
Contributor Author

@michalsn I caught BC 😄
Let's say there is a "Show deleted" checkbox in the list of users. After clicking, HTMX changes the URI in the browser, but not in the history. If we click another button/link and try to return to the current URI (as previous_url()), it will turn out to be the old one (before clicking the "Show deleted" button)
Now I fully understand your pagination example.

So I haven't completely solved my problem.

@michalsn
Copy link
Owner

michalsn commented Dec 6, 2024

What is your problem? Sometimes you want to store the previousURL even if it's disabled?
Or are you referring to the browser history?

@neznaika0
Copy link
Contributor Author

neznaika0 commented Dec 6, 2024

The example with pagination is simpler:

  1. Go to /users
  2. Click to pages Next/Back from HTMX and stop at /users?page=10
  3. Click on the button with HTMX /users/ban/33 and return to the same page (via previous_url()) Ops.. We should go back to /users?page=10 but the story is disabled - we get to /users

Now I have forgotten the exact example of the initial problem. But apparently this is not how it was planned.

EDIT: A good solution is to use HX-Current-Url as it sends the URL of the start page. It is similar to the Referer but looks more reliable

@michalsn
Copy link
Owner

michalsn commented Dec 6, 2024

Why not use ajax-header extension for places where you want don't want to store the URL?

@neznaika0
Copy link
Contributor Author

Let's leave this question for now. When I find the exact example, I'll come back.

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.

bug: Saving the previous URL
2 participants