Soft deprecate config labels #1300
Conversation
| }); | ||
| const field = config.create( | ||
| { | ||
| ...(config.label ? { label: config.label } : {}), |
There was a problem hiding this comment.
applyDefaults removed here as some long time ago, we made the change to serialize empty labels to '' instead of undefined (so that they would persist across loads). Hence this wasn't doing anything to properly formatted schemas.
| const defaults = {}; | ||
| set(defaults, DATETIME_SUBTYPE_PATH, DATETIME_SUBTYPES.DATE); | ||
| set(defaults, DATE_LABEL_PATH, 'Date'); | ||
|
|
||
| if (isNew) { |
There was a problem hiding this comment.
Here we need isNew so that date label doesn't get created for time components. It's the only place where this is required.
| group: 'basic-input', | ||
| emptyValue: null, | ||
| sanitizeValue: sanitizeDateTimePickerValue, | ||
| create: (options = {}) => { | ||
| create: (options = {}, isNew) => { |
There was a problem hiding this comment.
I just think we should give a more descriptive name
| create: (options = {}, isNew) => { | |
| create: (options = {}, is NewLabel) => { |
There was a problem hiding this comment.
I'll rename it all to isNewField (the fact that the label is new is only an implementation detail of the fact that the field is new).
| @@ -13,7 +13,7 @@ export class FieldFactory { | |||
| this._formFields = formFields; | |||
| } | |||
|
|
|||
| create(attrs, applyDefaults = true) { | |||
| create(attrs, isNew = true) { | |||
There was a problem hiding this comment.
| create(attrs, isNew = true) { | |
| create(attrs, isNewLabel = true) { |
| @@ -302,12 +310,12 @@ describe('core/FieldFactory', function () { | |||
|
|
|||
| // helpers ////////////// | |||
|
|
|||
| function testCreate(options, applyDefaults = true) { | |||
| const { type, label, keyed = false, defaults = {} } = options; | |||
| function testCreate(options, isNew = true) { | |||
There was a problem hiding this comment.
| function testCreate(options, isNew = true) { | |
| function testCreate(options, isNewLabel = true) { |
Related to #1291
Closes #1291
I ended up trying to keep it minimal. Still keeping the isNew flag, but there's only one place where it HAD to be used. We are safe to default the labels now because in any case, if it's not new, the label should be '' (not undefined).