Skip to content

fix(carbon): pass the label for numeric text-field correctly #949

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

Merged

Conversation

skateman
Copy link
Member

@skateman skateman commented Jan 21, 2021

The TextInput component takes a labelText prop, while the NumberInput needs a label 😕 😞

Before:
Screenshot from 2021-01-21 16-37-23

After:
Screenshot from 2021-01-21 16-37-17

@rvsia
Copy link
Contributor

rvsia commented Jan 21, 2021

@skateman We have a test file to catch up these issues: https://github.com/data-driven-forms/react-forms/blob/master/packages/carbon-component-mapper/src/tests/components.test.js

Could you please update it with this new variant?

  1. add new component to component mapper under some key (NumberInput) and use globalProps to set up type: 'number'
  2. add the key to the list
  3. profit 💰

@codecov
Copy link

codecov bot commented Jan 21, 2021

Codecov Report

Merging #949 (d221200) into master (49aec5e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #949   +/-   ##
=======================================
  Coverage   93.36%   93.37%           
=======================================
  Files         235      235           
  Lines        3710     3711    +1     
  Branches     1239     1240    +1     
=======================================
+ Hits         3464     3465    +1     
  Misses        246      246           
Impacted Files Coverage Δ
...es/carbon-component-mapper/src/files/text-field.js 100.00% <100.00%> (ø)

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 778ed8b...d221200. Read the comment docs.

@skateman skateman force-pushed the carbon-fix-label-numeric branch 5 times, most recently from f75e3a7 to 1a9b750 Compare January 22, 2021 13:05
@skateman skateman force-pushed the carbon-fix-label-numeric branch from 1a9b750 to d221200 Compare January 22, 2021 14:58
@skateman
Copy link
Member Author

@rvsia @Hyperkid123 can we get this in as it is or could you please help me with the tests?

Copy link
Member

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will deal with the tests later. We don't have the capacity right now. So we can merge them and add them later on.

@Hyperkid123 Hyperkid123 merged commit b54381a into data-driven-forms:master Jan 26, 2021
@Hyperkid123
Copy link
Member

🎉 This PR is included in version 2.20.3 🎉

The release is available on

Demo can be found here!

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

Successfully merging this pull request may close these issues.

3 participants