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

Add favicon to published Storybook #217

Closed
wants to merge 3 commits into from
Closed

Add favicon to published Storybook #217

wants to merge 3 commits into from

Conversation

fredvisser
Copy link
Contributor

@fredvisser fredvisser commented Dec 3, 2021

Pull Request

🤨 Rationale

Blocked: I'm going to wait on new icons, before supporting all the browsers in all the ways.

Both local and published versions of Storybook didn't include a custom favicon. While the icon is likely to change in the coming weeks, this PR takes the existing logo and uses it as a favicon.

👩‍💻 Implementation

Imported and specified the favicon within the existing .storybook/manager.js file.
Also, updated the old logo used within the top-level nimble readme.

🧪 Testing

Confirmed the favicon appears correctly in Firefox, Chrome, Safari when hosted locally.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

import theme from './theme';

const link = document.createElement('link');
Copy link
Member

Choose a reason for hiding this comment

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

Does an approach like this work: storybookjs/storybook#6155 (comment)

Pretty sure the js based approach won't work very generally (search indexing, bookmarks in browsers)

@@ -0,0 +1,7 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

We should also update the example client app favicon

@fredvisser fredvisser closed this Feb 1, 2022
@rajsite
Copy link
Member

rajsite commented Feb 1, 2022

@fredvisser still need the branch?

nate-ni added a commit that referenced this pull request Sep 6, 2022
# Pull Request

## 🤨 Rationale
The favicon in the Storybook site was non existent. Updating it to the Nimble icon.

Used this information to help direct me to updating it:
[How to change favicon? (Storybook GitHub)](storybookjs/storybook#6155)
[Reference example from the MS Fluent Storybook Repo](https://github.com/microsoft/fluentui/blob/4c970883a796505b132b356aa03302c068eb0ea9/packages/web-components/.storybook/manager-head.html)

I think this replaces this closed pull request: #217 

## 👩‍💻 Implementation
- Added ICO and PNG versions of the Nimble Icon
- Created an HTML file to reference them as the favicon
- Updated the npm command 'storybook' to add the image files directory

## 🧪 Testing
Ran storybook locally to make sure it displays the favicon correctly.

<img width="871" alt="image" src="https://user-images.githubusercontent.com/109235103/187926080-0cedfec7-ea43-4453-a316-69f36ba0ddbf.png">

<img width="481" alt="image" src="https://user-images.githubusercontent.com/109235103/187926268-c222d54b-1eb6-420c-8d09-b249e8b5f368.png">



## ✅ Checklist
- [x ] I have updated the project documentation to reflect my changes or determined no changes are needed.
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.

2 participants