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

docs: Fix how-it-works, add favicon+logo #710

Merged
merged 3 commits into from
Aug 4, 2024

Conversation

DeaMariaLeon
Copy link
Member

@DeaMariaLeon DeaMariaLeon commented Aug 3, 2024

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

  • Related issue #
  • Closes #

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below.

  • Some examples on "How it works" had disappeared when markdown-exec settings were added (I think).
  • Added back-to-top button when user scrolls up.
  • Added favicon and icon (these can be changed later of course). The icon file must be copied to the root as apple-touch-icon.png for macs to find it.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 3, 2024
Copy link
Member

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the issue!

CatDolphinGIF

Copy link
Member

@FBruzzesi FBruzzesi Aug 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a mac to try this, but that's new to me. I can't find it in any mkdocs-material docs either πŸ˜•

Copy link
Member Author

@DeaMariaLeon DeaMariaLeon Aug 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was building the docs locally, there was a 404 response when the favicon was supposed to be loaded. (I have a mac):

WARNING -  [13:54:22] "GET /apple-touch-icon-precomposed.png HTTP/1.1" code 404
WARNING -  [13:54:22] "GET /apple-touch-icon.png HTTP/1.1" code 404

So I saw this.. And now it works:

Screenshot 2024-08-04 at 13 59 41

But it's true that it looks weird to have that copy of the .png file there. I don't know if it will work on GitHubPages. I can remove it if you think I should.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible I would give it a try without it. I never had to have a duplicate when deploying to github pages

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

I never had to have a duplicate when deploying to github pages

The issue happens with a mac and safari. At least for me. Maybe my mac is broken. πŸ€”

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed the docs are live, how does it look from safari?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @FBruzzesi, the favicon is not found.

@DeaMariaLeon
Copy link
Member Author

I forgot: I can change the favicon & icon. I didn't use Magdalena's logos on discord because I wasn't sure that people agreed on one. Or I can remove them all together.

@DeaMariaLeon DeaMariaLeon merged commit 4353e47 into narwhals-dev:main Aug 4, 2024
20 checks passed
@DeaMariaLeon DeaMariaLeon deleted the mkdoc branch August 4, 2024 17:04
aivanoved pushed a commit to aivanoved/narwhals that referenced this pull request Aug 6, 2024
* Fix how-it-works, add favicon+logo

* icon for macs

* removed extra .png file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants