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

(bug) Fix Select array values showing k (fixes #4560) #4586

Merged
merged 2 commits into from
Oct 26, 2018

Conversation

diagramatics
Copy link
Contributor

Issue: #4560

Turns out the assignment of k in a shorthand notation of an object was
wrong. Using k instead of [k]: k makes it assign to parameter key
'k' as opposed to what k as a variable contains.

This is testable with Jest. The added test covers this bug.

@storybook-safe-bot
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented Oct 26, 2018

Codecov Report

Merging #4586 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4586      +/-   ##
==========================================
+ Coverage   35.57%   35.59%   +0.01%     
==========================================
  Files         557      557              
  Lines        6732     6732              
  Branches      884      884              
==========================================
+ Hits         2395     2396       +1     
+ Misses       3877     3876       -1     
  Partials      460      460
Impacted Files Coverage Δ
addons/knobs/src/components/types/Select.js 84.61% <100%> (+7.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca1cd1e...8674380. Read the comment docs.

@ndelangen
Copy link
Member

I was planning on fixing this today, thank you so much @diagramatics! 🙇

@ndelangen ndelangen added this to the v4.0.0 milestone Oct 26, 2018
@ndelangen ndelangen merged commit 2d49c7d into storybookjs:master Oct 26, 2018
@ghost
Copy link

ghost commented Oct 29, 2018

@diagramatics @ndelangen it still does not work on 4.0.0

@chrisbutler
Copy link
Contributor

would be great to get this into a release!

@Drapegnik
Copy link

@chrisbutler, check out #4560 (comment)

@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Oct 31, 2018
@shilman
Copy link
Member

shilman commented Oct 31, 2018

@chrisbutler @Drapegnik @DanielG2002 missed this. will get out another release today

@ndelangen please tag with patch when you merge if it needs to go out in a release -- helps make sure things get cherry-picked 👍

shilman pushed a commit that referenced this pull request Oct 31, 2018
(bug) Fix Select array values showing k (fixes #4560)
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Oct 31, 2018
@Hypnosphi
Copy link
Member

weird, this was merged before 4.0 release

@shilman
Copy link
Member

shilman commented Nov 1, 2018

@Hypnosphi I released from the release/4.0 branch, which was probably snapshotted just before this got merged into master. My bad for not communicating better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: knobs bug patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants