-
Notifications
You must be signed in to change notification settings - Fork 3
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
Created by visualize link points to Chart #2043
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thanks for the enhancement @noahonyejese i believe this will need to be on TEST before it can be tested. I tested in the PR with this chart: https://visualization-tool-git-fix-visualize-link-to-public-charts-ixt1.vercel.app/en/v/cZhzddlopnkb?dataSource=Prod the target link i see is https://visualize.admin.ch/en/v/cZhzddlopnkb (which looks correct - as this suggests it points to a chart and not the landing page) but when I click the link I get a 404 - this page could not be found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @noahonyejese, LGTM!
I think it would be good to consider two comments related to state.key
, but other than that really nice :)
I wouldn't introduce dynamic URLs, it would only make it work for testing environments, which aren't supposed to hold shareable charts anyway.
We'd also potentially introduce another dependency to Vercel, which I don't think we really need 👍
Closes #2031
What's new?
This PR Now navigates users to the corresponding Chart when clicked on the "Created by visualize.admin.ch" section in the footer, like this users can benefit from quick chart creation & potentially starting out with a more established Chart.
How to test
Xx3-xKHq9K2G
) -> this is necessary because we only support production charts@bprusinowski please let me know if it makes sense here to make the URL dynamic or if I should keep it only for visualize.admin.ch, Vercel also has a URL called VERCEL_URL we could be using this for cases like these. Let me know what you think. 👀