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

Invert screenshot for dark color theme #1031

Closed
wants to merge 2 commits into from

Conversation

oberrich
Copy link

Checklist

  • Did none of the above

What changes does this Pull Request introduce?

This inverts the screenshots on the README of this repo.

Why is this necessary?

My eyes, bug prevention

For comparison see patch-1 vs. master. You may want to change the URL of the dark mode logo so it doesn't go away when I delete the patch-1 tag.

@jelmer
Copy link
Member

jelmer commented Jun 17, 2024

Please include the assets in the repo, rather than outside of it.

I guess doing this adds to the burden of updating the screenshot if we need to refresh it in the future. Not a big deal, but worth pointing out.

@oberrich
Copy link
Author

oberrich commented Jun 17, 2024

I don't think the screenshot is hosted inside the repo at least I couldn't find it.
If I copy & paste the image directly into my clone of this repository it uploads it to github.com/oberrich, so I guess the best place I can host this image for you is right here:
image

@ix5
Copy link
Member

ix5 commented Jun 20, 2024

The screenshot not being hosted inside the repo is due to me updating it from an external source (posativ.org) to one uploaded to the corresponding PR: #867

That wasn't the correct way to do it, we should add the light (and potentially dark mode) screenshots to this repo and link them from inside the markdown. I think GitHub's markdown parser can resolve (relative) links to files inside the current repo so that we don't have to hardcode the path to user-images.githubusercontent.com (example: ![Isso in action](screenshot.png)). That way, forks updating the screenshot files inside their repo don't have to wonder why the README image doesn't update.

Then again, if we want the images displayed correctly on PyPI, we need hardcoded paths, no?
Probably something like https://raw.githubusercontent.com/isso-comments/isso/master/isso/img/isso.svg is what we want, then.

As for re-generating the screenshots: I've tried to document that a bit inside https://github.com/isso-comments/isso-marketing-material but maybe someone can submit a PR to add a Dockerfile to automate that step.

@oberrich
Copy link
Author

oberrich commented Jun 23, 2024

isso/img/ and absolute URL is probably most reasonable and portable.

@ix5
Copy link
Member

ix5 commented Jun 30, 2024

isso/img/ and absolute URL is probably most reasonable and portable.

Sounds good. Please add a commit that places both images (light and dark) there and update the README (in another commit) to reference them.

Would also be nice to have a quickly reproducible way to (automatically) regenerate the screenshots.

@oberrich
Copy link
Author

isso/img/ and absolute URL is probably most reasonable and portable.

Sounds good. Please add a commit that places both images (light and dark) there and update the README (in another commit) to reference them.

Would also be nice to have a quickly reproducible way to (automatically) regenerate the screenshots.

Feel free to commit it on your own, I've decided not to spend any more time on this issue.

@oberrich oberrich closed this by deleting the head repository Sep 19, 2024
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