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

Remove text transform uppercase for knob labels #991

Merged
merged 3 commits into from
May 8, 2017
Merged

Remove text transform uppercase for knob labels #991

merged 3 commits into from
May 8, 2017

Conversation

cnandreu
Copy link
Contributor

@cnandreu cnandreu commented May 3, 2017

Issue:

What I did

Removed text-transform: uppercase from knob labels.

How to test

Create a knob that's camel case (e.g. boolean("isCollapsed", false)). It should render isCollapsed instead of ISCOLLAPSED.

FYI

I found this PR that does the same thing:

"Remove UPPERCASE text-transform from labels #53"
storybook-eol/storybook-addon-knobs#53
but it looks like the storybooks/storybook-addon-knobs repository is now deprecated.

I saw this related issue on this repository:

"Remove UPPERCASE text-transform from labels #793"
#793

with a comment saying:

A PR fixing this was merged couple days ago.

but I still see text-transform: 'uppercase' on master:
https://github.com/storybooks/storybook/blob/master/packages/addon-knobs/src/components/PropField.js#L22

Sorry if this is a duplicate of something else I didn't find.

@codecov
Copy link

codecov bot commented May 3, 2017

Codecov Report

Merging #991 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #991      +/-   ##
=========================================
+ Coverage    12.6%   12.6%   +<.01%     
=========================================
  Files         192     192              
  Lines        4427    4426       -1     
  Branches      707     707              
=========================================
  Hits          558     558              
+ Misses       3244    3243       -1     
  Partials      625     625
Impacted Files Coverage Δ
packages/addon-knobs/src/components/PropField.js 10.41% <ø> (+0.21%) ⬆️

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 2f27e4a...be455cc. Read the comment docs.

@aaronmcadam
Copy link
Contributor

What's the motivation behind this change?

@cnandreu
Copy link
Contributor Author

cnandreu commented May 4, 2017

What's the motivation behind this change?

I want to have a 1 to 1 mapping between props and knob labels. It's harder to read camel case props if you uppercase them (e.g. ISCOLLAPSABLECHILD vs isCollapsableChild). Now there's no way to make them preserve casing, but with my PR you can choose to pass the label with all uppercase characters if you want it that way (e.g. 'boolean("LABEL, false)').

@cnandreu
Copy link
Contributor Author

cnandreu commented May 8, 2017

@aaronmcadam do you know if there's someone I can tag to get this merged (or have an additional code review and changes, if necessary)?

@aaronmcadam
Copy link
Contributor

What do you think @ndelangen? Is this good to merge?

@ndelangen ndelangen merged commit 5affcbd into storybookjs:master May 8, 2017
@shilman shilman added the misc label May 27, 2017
@ndelangen ndelangen added the maintenance User-facing maintenance tasks label May 27, 2017
Copy link

nx-cloud bot commented May 22, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit be455cc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants