-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
feat: Improve styling in the addons panel #437
Merged
dannyhw
merged 24 commits into
storybookjs:next-6.0
from
jonathanj:improve-addons-styling
Mar 1, 2023
Merged
feat: Improve styling in the addons panel #437
dannyhw
merged 24 commits into
storybookjs:next-6.0
from
jonathanj:improve-addons-styling
Mar 1, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The panels are part of the Storybook UI, not the story content, they should always take safe area into account so that their UI isn't accidentally rendered in the margins of the device's screen.
KeyboardAvoidingView renders too much padding when the keyboard is open, which ends up pushing the padding of the view up beyond the keyboard. This is because the bottom inset (safe area inset + nav bar) is not taken into account.
This deprecates all of the previous theme values and re-organises the theme in a way that the majority of the Storybook UI can be themed this way.
This also renames the components something more intentional than `Button`.
This makes it convenient to return the correct type for the theme.
- Allow tapping anywhere on a collapsed section to expand it - More compact representation for values
This means users can override only the parts of the theme they're interested in, from `getStorybookUI`.
dannyhw
reviewed
Mar 1, 2023
dannyhw
approved these changes
Mar 1, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is really great, amazing work @jonathanj!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue:
The addons panel (and addons in general) are missing polished UI styling, as well as being inconsistently styled with aspects like differing paddings/margins between various addons.
What I did
The largest change is restructuring the theme, the goal is to have enough semantic values to meaningfully style the UI well enough for the default Storybook theme, so that users have as much control over their own Storybook's appearance. Once this was done, it was a matter of replacing hardcoded values across the various pieces of UI in the app and addons.
The theme changes are documented in the "Migrating from 5.3x to 6.5.x" section under "Theming" in MIGRATION.md. I think it might be worth writing a separate piece of (non-migration) documentation around theming, what do you think?
Some motivation for the aesthetic changes:
A goal for the aesthetic changes was to try keep the styling relatively low-key to avoid drawing attention from the story content itself.
There is a fair amount of duplication between addons and the main app, mostly because there's not really a mechanism to reuse components (like inputs and buttons) between addons and the main app. As we discussed, that is probably a task better left for #181 and the duplication can be cleaned up at a later stage.
A summary of the git commit log:
KeyboardAvoidingView
padding intruding on the app's UI (focus an input and summon the soft-keyboard to see padding creep up from the bottom)Screenshots
How to test
The behaviour of Storybook should be identical to how it was before, any significant changes to the behaviour would be a regression. I've added some additional stories to help test aspects such as all of the controls in one place (like the knobs example). There is a commented section in
examples/expo-example/.storybook/Storybook.tsx
with some partial theme overrides.I removed all the
theme.value || 'some_default'
patterns from the code, as this is a fair amount of maintenance overhead and in my testing I couldn't actually trigger a situation wheretheme.value
returned nothing when it ought to have returned something. Because the user's theme is deep-merged with the default theme, there shouldn't be a case of accidentally missing values.