feat: carousel integration with history navigation

[aegroto]

Description

Carousel's modal now closes when navigating backward through the browser's history, going back to the post instead of returning to the main page. Fixes #1508.

Screenshots

I made no screenshots but it should be pretty easy to test. I can make a video if needed.

Additional Context

I wasn't able to use router.push as suggested in the original issue and had to stick to the native APIs, which is documented by NextJS as an alternative to router methods as well.

Checklist

Are your changes backwards compatible? Please answer below: Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below: 8, I tested the changes manually and with various content.

For frontend changes: Tested on mobile, light and dark mode? Please answer below: Yes

Did you introduce any new environment variables? If so, call them out explicitly here: No

[ekzyis]

This isn't a shallow push. On backwards navigation, this closes the carousel by reloading the page. We do not want to reload the page, we only want to close the carousel on backwards navigation.

Mhh, on closer inspection, maybe this doesn't trigger a page reload on backward navigation so this could be fine. However, I am still curious why you couldn't use next/router.

I wasn't able to use router.push as suggested in the original issue

Why not?

---

You also need to pop the state again on close since else, manually closing an image will mean that your next backward navigation will do nothing except popping that stale state.

So if I open many images and close all of them myself, I need to click back as many times as I opened images before I actually go back.

[aegroto]

I have not been able to use router.push because it was not updating the history anyway. I don't know the reason, but I believe it may be related to the path being identical to the last one, and I guess the router drops duplicate entries. An alternative would be to use a different URL, but this hinders the modal to be opened, I still have to investigate this behaviour.

I didn't understand that the push had to be unique for the entire carousel. I will work on this as soon as possible.

[huumn]

Adding a query param might work:

router.push({ pathname: router.pathname, query: { carousel: true, ...router.query}}, router.asPath, { shallow: true }}

[aegroto]

That's what I've been trying to do but it looks like the carousel is immediately closed after a 'router.push' or 'router.replace' call. This does not happen when calling native `window' APIs though.

[huumn]

If you used shallow: true and it did that, then I'd guess something is rerendering that shouldn't.

[aegroto]

I believe this is due to line 73 of modal.js:

router.events.on('routeChangeStart', maybeOnClose)

The push call triggers this event, which then calls maybeOnClose. This makes sense as the native window API is not trigger router's events.

[aegroto]

I have been exploring this issue in a while and I confirm that the reason router.push is not viable is line 73 of modal.js:

router.events.on('routeChangeStart', maybeOnClose)

On the other hand, I have not found any solid way to solve it using native APIs either. An idea may be to track state pushes and add a trigger on back events, doing as much backs as pushes caused by carousel's opening. However, this looks hacky.

@ekzyis @huumn I believe that the best thing to do should be to re-engineer how the carousel works, may using Parallel routes or, more simply, giving control to the parent component over the modal's show/hide mechanism, avoiding the involve of routing. Let me know your thoughts about this, also because if this issue needs a rewrite I kindly ask you to consider upgrading it from easy to medium.

[huumn]

We aren't using the app router yet, so you'll need to reference the pages docs. I checked out your solution and it doesn't work on the first attempt for some reason (second attempt works though). I'll take a closer look when I get time. This shouldn't require a rewrite.

[gitguardian]

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

---

^{_{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}}

[aegroto]

I have committed a version of the fix which works in case of multiple carousel shows as well:

recording.mp4

The code overrides the close function of the carousel, triggering a 'back' navigation when the carousel is closed. It is still relying on native history APIs, unfortunately a router's shallow navigation immediately closes the carousel, presumably due to the listener I have highlighted in the previous comments.

Before marking this PR ready for review I would like to know if it's fine for your to use window's native APIs, then maybe explore a solution which relies on router in the future.