Improve back button behaviour #120

glyph · 2022-06-21T11:24:29Z

glyph commented

2022-06-21 11:24:29 +00:00

This is a bug fix for #92

I've used a cookie named back_url to allow for dynamic setting of the in-UI back button link (href tag) when visiting a page to which there are multiple possible paths. If the cookie is not set then the code falls back to a hardcoded path for the back button.

An example to illustrate:

The Scuttlebutt Settings page can be accessed by going Home -> Status -> Scuttlebutt Settings (via the little cog icon in the top-right) or Home -> Settings -> Scuttlebutt Settings. This means that the back button on Scuttlebutt Settings should either return the user to State or Settings, depending on the path they took to get there.

When the Status page is visited, the back_url cookie is set with a value of /status/scuttlebutt. A request to /settings/scuttlebutt results in a check of the back_url cookie; if a value is set, we define the back button link accordingly (ie. as /status/scuttlebutt) otherwise we set it as /settings.

It's not a perfect solution but it seems to work well for the example above, as well as in the case of the Profile page (which can be accessed via the Search page; Friends, Follows and Blocks pages; as well as the Homepage).

This is a bug fix for https://git.coopcloud.tech/PeachCloud/peach-workspace/issues/92 I've used a cookie named `back_url` to allow for dynamic setting of the in-UI back button link (`href` tag) when visiting a page to which there are multiple possible paths. If the cookie is not set then the code falls back to a hardcoded path for the back button. An example to illustrate: The Scuttlebutt Settings page can be accessed by going Home -> Status -> Scuttlebutt Settings (via the little cog icon in the top-right) or Home -> Settings -> Scuttlebutt Settings. This means that the back button on Scuttlebutt Settings should either return the user to State or Settings, depending on the path they took to get there. When the Status page is visited, the `back_url` cookie is set with a value of `/status/scuttlebutt`. A request to `/settings/scuttlebutt` results in a check of the `back_url` cookie; if a value is set, we define the back button link accordingly (ie. as `/status/scuttlebutt`) otherwise we set it as `/settings`. It's not a perfect solution but it seems to work well for the example above, as well as in the case of the Profile page (which can be accessed via the Search page; Friends, Follows and Blocks pages; as well as the Homepage).

glyph added the

bug

peach-web

labels 2022-06-21 11:24:29 +00:00

glyph added 4 commits 2022-06-21 11:24:31 +00:00

a6f52ce384 add cookie utils module for requests and responses

5bd8a68ddf set, retrieve and reset back_url cookies

a60d892e95 bump the patch version

continuous-integration/drone/pr Build is passing Details

65b5f95a90 update lockfile

glyph requested review from notplants 2022-06-21 11:24:42 +00:00

notplants commented

2022-06-22 14:24:32 +00:00

@glyph have you looked into using the javascript history object to achieve this functionality? http://www.exforsys.com/tutorials/javascript/javascript-history-object-properties-and-methods.html

This might be a more standard and flexible way to achieve this, in particular using history.back() when you click the back button might be the simplest way.

This has the benefit that it could handle a chain of back presses, which the cookie value back_url would not handle exactly as expected I think.

@glyph have you looked into using the javascript history object to achieve this functionality? http://www.exforsys.com/tutorials/javascript/javascript-history-object-properties-and-methods.html This might be a more standard and flexible way to achieve this, in particular using history.back() when you click the back button might be the simplest way. This has the benefit that it could handle a chain of back presses, which the cookie value back_url would not handle exactly as expected I think.

notplants commented

2022-06-22 14:29:49 +00:00

if we don't want the back button to ever take you away from peach UI,
could also imagine a version where "if the latest item in history is a peach page, use that as the href for the back button, and if not, then use the hardcoded fallback"

if we don't want the back button to ever take you away from peach UI, could also imagine a version where "if the latest item in history is a peach page, use that as the href for the back button, and if not, then use the hardcoded fallback"

glyph commented

2022-06-22 15:50:36 +00:00

@notplants

I briefly considered the JavaScript option but wanted to try a non-JS implementation first. I quite like the idea of adding JS for this feature but leaving the underlying behaviour in place, such that we still have decent behaviour if someone is using the UI with JS disabled.

I'll merge this PR ff you're happy with the code, documentation and approach taken and then I'll create a separate PR for the JS functionality.

@notplants I briefly considered the JavaScript option but wanted to try a non-JS implementation first. I quite like the idea of adding JS for this feature but leaving the underlying behaviour in place, such that we still have decent behaviour if someone is using the UI with JS disabled. I'll merge this PR ff you're happy with the code, documentation and approach taken and then I'll create a separate PR for the JS functionality.

notplants commented

2022-06-22 16:51:49 +00:00

@glyph aa cool, I didn't realize before that you were able to implement the cookie method entirely without javascript, thats interesting. In that case, yes sounds good to merge this one, which works for now and as a fallback, and then make a javascript version if inspired in a separate PR.

👍 1

notplants commented

2022-06-22 16:54:47 +00:00

code-wise, it would be cool if there were a way to add .add_cookie("back_url={current_url}") for all routes, so it didn't need to be hardcoded for all routes. With other web frameworks I've enjoyed having a sort of wrapper function for all routes where I would put this type of common logic, but I haven't looked into Rouille to know how one would do this. (not a blocker, just a comment)

glyph commented

2022-06-23 09:16:20 +00:00

@notplants

code-wise, it would be cool if there were a way to add .add_cookie("back_url={current_url}") for all routes, so it didn't need to be hardcoded for all routes.

That's a neat idea. It could be implemented like this:

(GET) (/scuttlebutt/blocks) => {
    let back_url_cookie = format!("back_url={}", request.raw_url());
    
    Response::html(routes::scuttlebutt::blocks::build_template())
    	.add_cookie(&back_url_cookie)
},

I do not know how (or if) it's possible to run this code automatically for each Response being generated. The fact that the author of Rouille created an "Add Response::with_cookie()" issue makes me think that the above approach is the way to go.

This whole back-button thing is quite interesting, since there are (at least) two different behaviour approaches: 1) return me to the previously visited page; 2) take me one step back towards the homepage. If the back_url cookie-based approach is implemented for all pages then clicking the back button several times would just bounce you back and forth between two pages 🙃

@notplants > code-wise, it would be cool if there were a way to add .add_cookie("back_url={current_url}") for all routes, so it didn't need to be hardcoded for all routes. That's a neat idea. It could be implemented like this: ```rust (GET) (/scuttlebutt/blocks) => { let back_url_cookie = format!("back_url={}", request.raw_url()); Response::html(routes::scuttlebutt::blocks::build_template()) .add_cookie(&back_url_cookie) }, ``` I do not know how (or if) it's possible to run this code automatically for each `Response` being generated. The fact that the author of Rouille created an ["Add `Response::with_cookie()`"](https://github.com/tomaka/rouille/issues/126) issue makes me think that the above approach is the way to go. This whole back-button thing is quite interesting, since there are (at least) two different behaviour approaches: 1) return me to the previously visited page; 2) take me one step back towards the homepage. If the `back_url` cookie-based approach is implemented for all pages then clicking the back button several times would just bounce you back and forth between two pages 🙃

glyph merged commit ab0e27c14d into main

2022-06-23 11:00:22 +00:00

glyph referenced this issue from a commit

2022-06-23 11:00:23 +00:00

Merge pull request 'Improve back button behaviour' (#120) from improved_back_button into main

glyph deleted branch improved_back_button

2022-06-23 11:00:24 +00:00

glyph referenced this pull request

2022-06-23 11:07:41 +00:00

Introduce JS for improved back button behaviour #121

glyph referenced this pull request

2022-06-27 07:31:45 +00:00

[BUG] Back button does not always behave accurately (redirects to incorrect page) #92

notplants commented

2022-06-27 14:59:58 +00:00

short-term memory back +1

notplants commented

2022-06-27 18:57:01 +00:00

@glyph here is a PR which has code to add the back_url_cookie to all private routes
#123

since handle_route returns a Response,
it would also be straight forward to add the cookie to public and private routes if deired... or to put whatever we want in handle_route... handle_route is the type of abstract function that wraps all responses that I was thinking of where one can add logic that applies to all routes, as opposed to individually

@glyph here is a PR which has code to add the back_url_cookie to all private routes https://git.coopcloud.tech/PeachCloud/peach-workspace/pulls/123 since handle_route returns a Response, it would also be straight forward to add the cookie to public and private routes if deired... or to put whatever we want in handle_route... handle_route is the type of abstract function that wraps all responses that I was thinking of where one can add logic that applies to all routes, as opposed to individually

glyph commented

2022-06-28 08:10:14 +00:00

@notplants

Oh nice, I hadn't thought to look at the handle_route() function. That's the perfect place for these kinds of blanket implementations.

That being said, I'm not keen on having the cookie-based back button for all routes because it creates a ping-pong effect which feels like bad UX to me. At least we have the JS solution now (#121 (comment)) and the current, mostly hierarchical fallback solution is good enough.

@notplants Oh nice, I hadn't thought to look at the `handle_route()` function. That's the perfect place for these kinds of blanket implementations. That being said, I'm not keen on having the cookie-based back button for all routes because it creates a ping-pong effect which feels like bad UX to me. At least we have the JS solution now (https://git.coopcloud.tech/PeachCloud/peach-workspace/pulls/121#issuecomment-13465) and the current, mostly hierarchical fallback solution is good enough.

notplants commented

2022-06-28 13:52:21 +00:00

my experience with software in general is the more little edge cases that have to be manually implemented in multiple places, the more bugs there will be, and especially since its a fallback anyway, I would lean towards not adding code complexity. I actually feel the same way about code comments which are similar/duplicate in multiple places (often they go stale, and while at first helpful, often eventually end up being misleading instead of helping). but also fair enough, it is mostly hierarchical and the ping pong behavior would be absurd (although also kind of entertaining)-- this is just the explanation of my holy war against code and comment duplication

notplants referenced this pull request

2022-06-29 20:26:30 +00:00

Add back_url_cookie for all private routes #123

Sign in to join this conversation.

No reviewers