search

remix-run / remix-website

322 stars 72 forks source link

Make pathname stay when version changes in docs #263

Closed nichtsam closed 3 months ago

nichtsam commented 3 months ago

Currently switching branches or version throws users back to the index page. I noticed that VersionWarningMessage preserves the path, so I just copied the logic over.

brookslybrand commented 3 months ago

Hey @nichtsam this is great! Thanks for adding this

I noticed that there's one small bug. I think once this is fixed I'm good to merge this in

https://github.com/remix-run/remix-website/assets/12396812/d5a859db-6cc5-4397-a891-7cc69102833e

nichtsam commented 3 months ago

Hey @nichtsam this is great! Thanks for adding this

I noticed that there's one small bug. I think once this is fixed I'm good to merge this in

https://github.com/remix-run/remix-website/assets/12396812/d5a859db-6cc5-4397-a891-7cc69102833e

I actually noticed the same thing, but the 'VersionWarningMessage' does this too, and I wasn't sure how to tackle this nicely.

Throwing user to the main page when it's 404 seems a bit rude and unclear. I thought at least 404 gives an idea to users that what they were looking doesn't exist.

nichtsam commented 3 months ago

Hey @nichtsam this is great! Thanks for adding this I noticed that there's one small bug. I think once this is fixed I'm good to merge this in

undefined_route.mov

I actually noticed the same thing, but the 'VersionWarningMessage' does this too, and I wasn't sure how to tackle this nicely.

Throwing user to the main page when it's 404 seems a bit rude and unclear. I thought at least 404 gives an idea to users that what they were looking doesn't exist.

Ah ok I didn't realize the bug in the url! 🤡 I'll look into this.

brookslybrand commented 3 months ago

Ah ok I didn't realize the bug in the url! 🤡 I'll look into this.

Haha sorry if I didn't make that clear. I imagine it's an easy fix, but ping me if it's not (or when you fix it!). Excited to merge 🙂

nichtsam commented 3 months ago

Ah ok I didn't realize the bug in the url! 🤡 I'll look into this.

Haha sorry if I didn't make that clear. I imagine it's an easy fix, but ping me if it's not (or when you fix it!). Excited to merge 🙂

@brookslybrand Fixed! It is an easy fix, I just didn't give the screen recording a good look 😂 I thought it was about the behavior if the doc doesn't exist after a version switch.