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

chore: tweak navigator design to match SB web navigator design #326

Merged
merged 3 commits into from
Mar 5, 2022

Conversation

a7medev
Copy link
Contributor

@a7medev a7medev commented Feb 11, 2022

Issue:

The old design was a bit unorganized and doesn't look like the Storybook web interface, so I tried to make it as close to it as possible.

What I did

I tweaked the navigator design to make it look like the one in the Storybook web interface.

Here's how it looks after the changes:

Storybook react-native navigator redesigned

How to test

You just need to run the example app and see the changes in action.

  • Does this need a new example in examples/native? No
  • Does this need an update to the documentation? No

NOTE: There're some issues with @emotion/native and TypeScript because I'm using the transform property which uses an array of objects in react-native instead of a string. If you have any recommendations on how to fix it please let me know.

NOTE: I didn't use images/SVGs for the grid icon, I did it with react-native views just to not include images or add dependency of react-native-svg to the package.

What's next?

I'll work on improving it in my free time, things I want to add:

  • Toggle component stories.
  • Add a bookmark icon beside the story name (like in the web version).
  • Add a search icon in the search input.
  • Improve the a11y of the navigator as a whole.
  • Focus on search input on / press.

@dannyhw
Copy link
Member

dannyhw commented Feb 11, 2022

@a7med-mahmoud hey thanks for your contribution 🙏 .

Looks cool! I'll test it out soon :)

@a7medev
Copy link
Contributor Author

a7medev commented Feb 14, 2022

I think one solution to the TypeScript problem is to upgrade Emotion to v11, what do you think, should I do so?

@dannyhw
Copy link
Member

dannyhw commented Feb 16, 2022

@a7med-mahmoud I think it would be better to update emotion separately.

If you think it would be best to do it first then we could merge a PR to update emotion first. Though we should check the version of emotion used by other storybook packages to make sure there are no clashes.

@dannyhw
Copy link
Member

dannyhw commented Feb 20, 2022

@a7med-mahmoud thanks again for this contribution. I was just having another look and you did a good job recreating the icon but I think it might be easier to use a png instead of a component. Then we can add all the other sidebar icons to the project too. What do you think?

story
component

@a7medev
Copy link
Contributor Author

a7medev commented Feb 20, 2022

If we will add icons as PNGs, I think it would be better in terms of consistency to convert the SVGs used in SB web to PNGs and use them instead.
I'll give it a try and share the progress with you

@dannyhw
Copy link
Member

dannyhw commented Feb 20, 2022

@a7med-mahmoud awesome thanks 🙏

The icons are component and bookmarkhollow

https://github.com/storybookjs/storybook/blob/next/lib/components/src/icon/icons.tsx

@a7medev
Copy link
Contributor Author

a7medev commented Feb 27, 2022

I added it as an image. Sorry for the delay, I didn't have much time to work on it.

@dannyhw
Copy link
Member

dannyhw commented Feb 28, 2022

@a7med-mahmoud

I added it as an image. Sorry for the delay, I didn't have much time to work on it.

Don't worry about delays, everything in its own time :). Again really appreciate your contribution and putting up with my requests 😛 .

Will take a look as soon as I can. Thanks again!

@dannyhw
Copy link
Member

dannyhw commented Mar 5, 2022

Great contribution @a7med-mahmoud !

Hope you don't mind I converted the images to base64 because otherwise it complicates the bundling of the package. Any non ts/js files need to be copied into the dist folder after compiling. As base64 its not needed.

I also added in the story icon :)

Thanks again! 🙇

Copy link
Member

@dannyhw dannyhw left a comment

Choose a reason for hiding this comment

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

@dannyhw dannyhw merged commit 950f82b into storybookjs:next-6.0 Mar 5, 2022
@dannyhw
Copy link
Member

dannyhw commented Mar 5, 2022

now live in 6.0.1-beta.5

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