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

fix: broken screenshot links in docs #2472

Merged
merged 2 commits into from
Oct 2, 2024
Merged

fix: broken screenshot links in docs #2472

merged 2 commits into from
Oct 2, 2024

Conversation

rparkr
Copy link
Contributor

@rparkr rparkr commented Oct 2, 2024

📝 Summary

Fixes broken image links in #2458.

I also masked the API key in the screenshots -- previously it was a fake API key, but resembled an actual key. Abbreviating the key with ... eliminates that confusion.

🔍 Description of Changes

  • I updated the src attributes in the <img> tags to point to the /_static directory (using an absolute path)
  • I verified that the images load correctly by building from source and running make ARGS="-a" docs as shown in the contributor guide (thanks for that helpful guide!). See the after screenshot below.
  • I updated the screenshots to mask out the API key. Previously it was a fake API key I generated using Python's uuid.uuid4(). Masking out the (fake) API key eliminates confusion.
  • I formatted the HTML tags in the markdown page to match the formatting used in other pages in the docs
  • I added figcaptions to the screenshots, following the pattern used in other areas of the docs

After
image

Before
image

Notes on dark-theme screenshots

If you'd prefer to use the dark-theme images in the screenshots instead, you can add -dark.png to the end of the src image paths; the dark-theme screenshot is also updated with the API mask. I chose the light-themed versions because the screenshots in other areas of the docs use a light theme.

Also, feel free to delete the dark-themed images if there isn't a use for them, since that would keep the _static directory smaller.

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • I have added tests for the changes made. (I did not make any changes to the code besides the docs)
  • I have run the code and verified that it works as expected.

📜 Reviewers

@mscolnick

rparkr added 2 commits October 1, 2024 22:37
- fix links to screenshots
- replace screenshots with updated versions that obfuscate the API key (which was a fake API key before, but still looked real. Now it's less confusing)
- Also, format the HTML to match formatting used in other docs pages
- Use absolute paths from the /_static directory so that links will work when the docs are built
Copy link

vercel bot commented Oct 2, 2024

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

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 2, 2024 4:24am
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 2, 2024 4:24am

@Haleshot
Copy link
Contributor

Haleshot commented Oct 2, 2024

Aha, thankful to come across this :) Was experimenting with Ollama models with marimo (in the AI Completion docs) and noticed the images rendering incorrectly as shown in the screenshots above!
Had noted it down as an issue myself (in my personal TODO) and was going to raise a PR for this today!

@rparkr
Copy link
Contributor Author

rparkr commented Oct 2, 2024

Aha, thankful to come across this :) Was experimenting with Ollama models with marimo (in the AI Completion docs) and noticed the images rendering incorrectly as shown in the screenshots above! Had noted it down as an issue myself (in my personal TODO) and was going to raise a PR for this today!

Thanks @Haleshot! I appreciate the encouragement :) I'm just getting started on my journey in open source contributions, and the encouragement means a lot.

@akshayka
Copy link
Contributor

akshayka commented Oct 2, 2024

Thanks so much @rparkr !

@akshayka akshayka merged commit a618b8e into marimo-team:main Oct 2, 2024
19 checks passed
Copy link

github-actions bot commented Oct 2, 2024

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.8.23-dev18

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.

3 participants