Switch from dynamic URLs to query string parameters for migration details #2235
Conversation
✅ Deploy Preview for conda-forge-previews ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Awesome! While we are here, is it possible to add navigation params to:
|
Yea I can't reproduce those in this PR either. |
Hm, should these be in the URL? I can see some value to it but I'm a little ambivalent. I wouldn't combine it with this PR, we can just add these features to the backlog. |
The issue I'm observing is that when I link to the migration page to new users, they are confused because initially there's no information displayed. They have to learn to click on the toggle buttons to display some stuff, and then they don't really know what statuses are representing. By having the categories in the URL I could link exactly to the part they need to pay attention to, instead of browsing / Ctrl+F'ing their package so much. But yea, it can be done in a separate PR; only mentioned it in case it was low hanging fruit. |
|
Why not just have everything toggled to on when the page loads? This seems more useful anyways. |
|
Another thing to consider, all the migration links that have been posted til now will start 404ing. Example https://deploy-preview-2235--conda-forge-previews.netlify.app/status/migration/aws_c_cal071. Is there a chance we can leave the navigation in place for a while so it redirects to the |
Yes, that's item 6 in #2137 (comment) |
This is orthogonal to whether we put them in the URL, and I think you're correct. @HaudinFlorence is looking at the meta list and that's one of the issues she'll be handling. |
I am not 100% what the syntax is, but I believe Docusaurus supports URL rewriting for this purpose and that's a configuration rather than application code thing. |
|
Right my point was that @jaimergp wants things in the URL so he can have them on when he sends links to confuse people less. If they are all on by default, then that would solve his problem partly and might alleviate the need for them in the links. |
They allow redirects from existing pages, but not just any arbitrary URL, IIRC. Anyway, I think it's not a big deal if it 404s, and we can also take a look separately if nobody else in core thinks it's a big problem. I'd leave this PR open through the weekend just to give an opportunity to voice their concerns if any, and then merge in Monday otherwise. WDYT? |
|
I took a look at how we could reimplement the redirects but it doesn't work really well (double renders, more 404 flashing etc). Instead, I decided to catch the 404 page and print something more useful for the end user. They will have to click a link but at least it's not as confusing as "ha! it's gone!" message. Visit https://deploy-preview-2235--conda-forge-previews.netlify.app/status/migration/aws_c_io01410 to get an idea: 
|
|
Let's give this a try. |
In order to prevent the 404 flash, we should stop using dynamically generated URLs for each migration details page and instead rely on a query string parameter. This PR fixes the 404 flash.
Fixes #2132
This issue is also mentioned in #2137 and fixes bullet points number 2 and number 4.
This PR (almost certainly) also fixes the issue in this comment.
Screenshot:
Note the URL structure.
cc: @HaudinFlorence @jaimergp