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 id generation in NumberField and TextField #92

Merged
merged 3 commits into from
Sep 25, 2018
Merged

Conversation

dferber90
Copy link
Contributor

The id generation when no id prop is provided was broken in NumberField and TextField. The tests were not detecting this either.

This PR fixes the generation, the tests and introduces do expressions to replace the iife.

Previously it could happen that no id was generated when id={undefined} was passed explicitly. The tests were not catching that and were upgraded as well.
Previously it could happen that no id was generated when id={undefined} was passed explicitly. The tests were not catching that and were upgraded as well.
@@ -8,7 +8,7 @@ states.
## Usage

```js
import TextField from '@commercetools-frontend/ui-kit/fields/text-field';
import { TextField } from '@commercetools-frontend/ui-kit';
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@dferber90 dferber90 merged commit 96151d8 into master Sep 25, 2018
@dferber90 dferber90 deleted the df-fix-fields branch September 25, 2018 13:41
lufego pushed a commit that referenced this pull request Oct 1, 2018
* docs(text-field): fix story

* fix(number-field): ensure id

Previously it could happen that no id was generated when id={undefined} was passed explicitly. The tests were not catching that and were upgraded as well.

* fix(text-field): ensure id

Previously it could happen that no id was generated when id={undefined} was passed explicitly. The tests were not catching that and were upgraded as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants