-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
FIX brandImage staying storybook's logo when theme is created with brandTitle #6120
Conversation
…to null if not supplied also
Codecov Report
@@ Coverage Diff @@
## next #6120 +/- ##
==========================================
- Coverage 37.64% 37.64% -0.01%
==========================================
Files 642 642
Lines 9390 9391 +1
Branches 1365 1337 -28
==========================================
Hits 3535 3535
Misses 5295 5295
- Partials 560 561 +1
Continue to review full report at Codecov.
|
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.
👍
FIX brandImage staying storybook's logo when theme is created with brandTitle
@@ -170,7 +170,7 @@ export const convert = (inherit: ThemeVars = lightThemeVars): Theme => { | |||
brand: { | |||
title: brandTitle, | |||
url: brandUrl, | |||
image: brandImage, | |||
image: brandImage || brandTitle ? null : undefined, |
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.
Doesn't this make brandImage never used? Shouldn't this be
image: brandImage || (brandTitle ? null : undefined),
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.
Good catch 😖
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.
@kennethtruong Mind doing a small PR for this?
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.
Sure!
Issue: #5866
What I did
CHANGE theme creation so when setting brandTitle, it sets brandImage to null if not supplied also.