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

Fix deprecationThemeOptions that always override final brand object with undefined #6522

Closed
wants to merge 1 commit into from

Conversation

lonyele
Copy link
Member

@lonyele lonyele commented Apr 15, 2019

Issue: #6521
Previous PR that made this issue: #5402 (comment)

What I did

I changed to what previous code does.

How to test

  • Is this testable with Jest or Chromatic screenshots? I'm not sure
  • Does this need a new example in the kitchen sink apps? No
  • Does this need an update to the documentation? No

Comments

Hi, deja-vue again... I found this while working on this PR.

As you can see from previous PR that made this issue, It has been not working from after PR is merged.
My understanding is when user inputs name, url, it maps its values to brandTitle, brandUrl with deprecation warnings. However previous code always warns(console.log) deprecationThemeOptions warning whether user inputs name or url and overrides brandTitle, brandUrl, brandImage to undefined, undefined, null. Thus whatever options passing from user doesn't do anything.

@vercel
Copy link

vercel bot commented Apr 15, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-lonyele-fix-render-brand-object.storybook.now.sh

@ndelangen
Copy link
Member

@lonyele can you verify the fix is still needed and if so resolve the merge conflict?

@lonyele
Copy link
Member Author

lonyele commented Apr 19, 2019

@ndelangen since #6543 is merged, I think this fix is not needed.

@lonyele lonyele closed this Apr 19, 2019
@lonyele lonyele deleted the fix/render-brand-object branch April 19, 2019 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants