Handle field placeholder when labels disabled #426
Conversation
janowsiany
changed the title
|
Should i put example of handling placeholder with GraphQLBridge extras? |
Codecov Report
@@ Coverage Diff @@
## master #426 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 157 157
Lines 1396 1401 +5
=====================================
+ Hits 1396 1401 +5
Continue to review full report at Codecov.
|
| @@ -137,6 +137,12 @@ export default class GraphQLBridge extends Bridge { | |||
| ready.label = ''; | |||
| } | |||
|
|
|||
| if (props.placeholder === true && extra.placeholder) { | |||
| ready.placeholder = extra.placeholder; | |||
| } else if (props.placeholder !== undefined && !props.placeholder) { | |||
There was a problem hiding this comment.
I'd rather check for false and null explicitly here.
| it('works with placeholder (falsy)', () => { | ||
| expect(bridgeI.getProps('id', {placeholder: null})).toEqual({ | ||
| label: 'Post ID', | ||
| placeholder: '', |
There was a problem hiding this comment.
Test for placeholder === true and extra.placeholder === undefined is missing.
| const ready = { | ||
| allowedValues, | ||
| ...(fieldType === 'number' ? {decimal: true} : {}), | ||
| options: opts || options, | ||
| label: label === true ? fieldName.charAt(0).toUpperCase() + fieldName.slice(1).toLowerCase() : label || '', | ||
| label: label === true ? fieldNameCapitalized : label || '', | ||
| placeholder: placeholder === true ? fieldNameCapitalized : placeholder || undefined, |
There was a problem hiding this comment.
There's no need for || undefined.
| @@ -139,7 +139,7 @@ export default class GraphQLBridge extends Bridge { | |||
|
|
|||
| if (props.placeholder === true && extra.placeholder) { | |||
| ready.placeholder = extra.placeholder; | |||
| } else if (props.placeholder !== undefined && !props.placeholder) { | |||
| } else if (props.placeholder !== undefined && (props.placeholder === false || props.placeholder === null)) { | |||
There was a problem hiding this comment.
Now the left part of && is not needed. Please remove it and we are all set.
Solves bug raised in #421