Skip to content
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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

noahonyejese
Copy link
Contributor

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

  1. Go to this Link
  2. Create a new Visualisation
  3. Publish Visualisation
  4. Memorize last part of the URL (e.g Xx3-xKHq9K2G) -> this is necessary because we only support production charts
  5. Click the "Created by visualize.admin.ch"
  6. See how it navigates to the same Chart (See url) ✅

@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. 👀

@noahonyejese noahonyejese self-assigned this Jan 30, 2025
Copy link

vercel bot commented Jan 30, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
visualization-tool ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2025 1:44pm

@noahonyejese noahonyejese changed the title feat: Added configKey as prop to VisualizeLink Created by visualize link points to Chart Jan 30, 2025
@sosiology
Copy link
Contributor

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.

Copy link
Collaborator

@bprusinowski bprusinowski left a 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 👍

app/components/chart-footnotes.tsx Show resolved Hide resolved
app/components/chart-preview.tsx Outdated Show resolved Hide resolved
app/components/chart-published.tsx Outdated Show resolved Hide resolved
app/components/chart-shared.tsx Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link "created with visualize.admin.ch" should point to the published chart
3 participants