[Protocol Monitor] Fix behavior for optional object parameters Screencast: https://screencast.googleplex.com/cast/NTQzMDU4MzQ4OTQ2MjI3Mnw4MzliNDg5NS0wYg Bug: 1473556 Change-Id: I9124921d8d6499e50c1403f953c5c035a5a5eeef Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4798130 Reviewed-by: Alex Rudenko <alexrudenko@chromium.org> Commit-Queue: Hadrien Jaubert <hadrienjaubert@google.com>
diff --git a/front_end/panels/protocol_monitor/ProtocolMonitor.ts b/front_end/panels/protocol_monitor/ProtocolMonitor.ts index 3ba8e65..f2e8065 100644 --- a/front_end/panels/protocol_monitor/ProtocolMonitor.ts +++ b/front_end/panels/protocol_monitor/ProtocolMonitor.ts
@@ -723,7 +723,6 @@ this.tabbedPane.show(this.contentElement); this.tabbedPane.selectTab('response'); this.request = {}; - this.targetId = ''; this.render(null); }
diff --git a/front_end/panels/protocol_monitor/components/JSONEditor.ts b/front_end/panels/protocol_monitor/components/JSONEditor.ts index a9c6917..2087187 100644 --- a/front_end/panels/protocol_monitor/components/JSONEditor.ts +++ b/front_end/panels/protocol_monitor/components/JSONEditor.ts
@@ -85,7 +85,7 @@ interface ObjectParameter extends BaseParameter { type: ParameterType.Object; - value: Parameter[]; + value?: Parameter[]; } export type Parameter = ArrayParameter|NumberParameter|StringParameter|BooleanParameter|ObjectParameter; @@ -463,9 +463,9 @@ return { ...parameter, - value: nestedParameters, + value: parameter.optional ? undefined : nestedParameters, isCorrectType: true, - }; + } as Parameter; } if (parameter.type === ParameterType.Array) { return { @@ -638,8 +638,7 @@ #handleAddParameter(parameterId: string): void { const pathArray = parameterId.split('.'); - const {parameter} = this.#getChildByPath(pathArray); - + const {parameter, parentParameter} = this.#getChildByPath(pathArray); if (!parameter) { return; } @@ -679,6 +678,9 @@ if (!typeRef) { throw Error('Every object parameter must have a typeRef'); } + if (!parameter.value) { + parameter.value = []; + } if (!this.typesByName.get(typeRef)) { parameter.value.push({ type: ParameterType.String, @@ -691,19 +693,27 @@ }); break; } - const nestedType = this.typesByName.get(typeRef) ?? []; + const nestedTypes = this.typesByName.get(typeRef) ?? []; const nestedValue: Parameter[] = - nestedType.map(nestedType => this.#createNestedParameter(nestedType, nestedType.name)); - - parameter.value.push({ - type: ParameterType.Object, - name: '', - optional: true, - typeRef: typeRef, - value: nestedValue, - isCorrectType: true, - description: '', + nestedTypes.map(nestedType => this.#createNestedParameter(nestedType, nestedType.name)); + const nestedParameters = nestedTypes.map(nestedType => { + return this.#populateParameterDefaults(nestedType); }); + + if (parentParameter) { + parameter.value.push({ + type: ParameterType.Object, + name: '', + optional: true, + typeRef: typeRef, + value: nestedValue, + isCorrectType: true, + description: '', + }); + } else { + parameter.value = nestedParameters; + } + break; } default: @@ -714,17 +724,21 @@ this.requestUpdate(); } - #handleClearParameter(parameter: Parameter): void { - if (!parameter) { + #handleClearParameter(parameter: Parameter, isParentArray?: boolean): void { + if (!parameter || parameter.value === undefined) { return; } switch (parameter.type) { case ParameterType.Object: + if (parameter.optional && !isParentArray) { + parameter.value = undefined; + break; + } if (!parameter.typeRef || !this.typesByName.get(parameter.typeRef)) { parameter.value = []; } else { - parameter.value.forEach(param => this.#handleClearParameter(param)); + parameter.value.forEach(param => this.#handleClearParameter(param, isParentArray)); } break; @@ -872,12 +886,15 @@ const isParentObject = parentParameter && parentParameter.type === ParameterType.Object; const isObject = parameter.type === ParameterType.Object; + const isParamValueUndefined = parameter.value === undefined; + const isParamOptional = parameter.optional; const hasTypeRef = isObject && parameter.typeRef && this.typesByName.get(parameter.typeRef) !== undefined; // This variable indicates that this parameter is a parameter nested inside an object parameter // that no keys defined inside the CDP documentation. const hasNoKeys = parameter.isKeyEditable; const isCustomEditorDisplayed = isObject && !hasTypeRef; const hasOptions = parameter.type === ParameterType.String || parameter.type === ParameterType.Boolean; + const canClearParameter = (isArray && !isParamValueUndefined && parameter.value.length !== 0) || (isObject && !isParamValueUndefined); const parametersClasses = { 'optional-parameter': parameter.optional, 'parameter': true, @@ -920,21 +937,30 @@ `: nothing} <!-- Render button to complete reset an array parameter or an object parameter--> - ${(isArray && parameter.value.length !== 0) || isObject ? + ${canClearParameter ? this.#renderInlineButton({ title: i18nString(UIStrings.resetDefaultValue), iconName: 'clear', - onClick: () => this.#handleClearParameter(parameter), + onClick: () => this.#handleClearParameter(parameter, isParentArray), classMap: {'clear-button': true}, }) : nothing} <!-- Render the buttons to change the value from undefined to empty string for optional primitive parameters --> - ${isPrimitive && !isParentArray && parameter.optional && parameter.value === undefined ? + ${isPrimitive && !isParentArray && isParamOptional && isParamValueUndefined ? html` ${this.#renderInlineButton({ title: i18nString(UIStrings.addParameter), iconName: 'plus', onClick: () => this.#handleAddParameter(parameterId), - classMap: { 'delete-button': true }, + classMap: { 'add-button': true }, + })}` : nothing} + + <!-- Render the buttons to change the value from undefined to populate the values inside object with their default values --> + ${isObject && isParamOptional && isParamValueUndefined ? + html` ${this.#renderInlineButton({ + title: i18nString(UIStrings.addParameter), + iconName: 'plus', + onClick: () => this.#handleAddParameter(parameterId), + classMap: { 'add-button': true }, })}` : nothing} </div> @@ -961,7 +987,7 @@ })}`: nothing} <!-- In case the parameter is not optional or its value is not undefined render the input --> - ${isPrimitive && !hasNoKeys && (parameter.value !== undefined || !parameter.optional) && (!isParentArray) ? + ${isPrimitive && !hasNoKeys && (!isParamValueUndefined || !isParamOptional) && (!isParentArray) ? html` <devtools-suggestion-input data-paramId=${parameterId} @@ -976,7 +1002,7 @@ ></devtools-suggestion-input>` : nothing} <!-- Render the buttons to change the value from empty string to undefined for optional primitive parameters --> - ${isPrimitive &&!hasNoKeys && !isParentArray && parameter.optional && parameter.value !== undefined ? + ${isPrimitive &&!hasNoKeys && !isParentArray && isParamOptional && !isParamValueUndefined ? html` ${this.#renderInlineButton({ title: i18nString(UIStrings.resetDefaultValue), iconName: 'clear',
diff --git a/test/unittests/front_end/panels/protocol_monitor/components/JSONEditor_test.ts b/test/unittests/front_end/panels/protocol_monitor/components/JSONEditor_test.ts index 6057d2c..2a880ba 100644 --- a/test/unittests/front_end/panels/protocol_monitor/components/JSONEditor_test.ts +++ b/test/unittests/front_end/panels/protocol_monitor/components/JSONEditor_test.ts
@@ -57,7 +57,7 @@ }, 'Test.test3': { parameters: [{ - 'optional': true, + 'optional': false, 'type': 'object', 'value': [ { @@ -125,7 +125,7 @@ { 'name': 'traceConfig', 'type': 'object', - 'optional': true, + 'optional': false, 'description': '', 'typeRef': 'Tracing.TraceConfig', }, @@ -154,6 +154,26 @@ 'typeRef': 'Test.arrayTypeRef', }], }, + 'Test.test12': { + parameters: [{ + 'optional': true, + 'type': 'object', + 'value': [ + { + 'optional': false, + 'type': 'string', + 'name': 'param1', + }, + { + 'optional': false, + 'type': 'number', + 'name': 'param2', + }, + ], + 'name': 'test12', + 'typeRef': 'Optional.Object', + }], + }, }, }, ] as Iterable<ProtocolMonitor.ProtocolMonitor.ProtocolDomain>; @@ -558,7 +578,7 @@ await jsonEditor.updateComplete; const inputs = jsonEditor.renderRoot.querySelectorAll('devtools-suggestion-input'); - // inputs[0] corresponds to the devtools-recorder-input of the command + // inputs[0] corresponds to the devtools-suggestion-input of the command const suggestionInput = inputs[1]; // Reset the value to empty string because for boolean it will be set to false by default and the correct suggestions will not show suggestionInput.value = ''; @@ -621,6 +641,53 @@ assert.deepStrictEqual(value, expectedValue); }); + + it('should show the keys with default values when clicking of plus button for optional object parameters', + async () => { + const command = 'Test.test12'; + const typesByName = new Map(); + typesByName.set('Optional.Object', [ + { + 'optional': false, + 'type': 'string', + 'name': 'param1', + }, + { + 'optional': false, + 'type': 'number', + 'name': 'param2', + }, + ]); + const jsonEditor = renderJSONEditor(); + jsonEditor.typesByName = typesByName; + await populateMetadata(jsonEditor); + jsonEditor.command = command; + jsonEditor.populateParametersForCommandWithDefaultValues(); + await jsonEditor.updateComplete; + + const param = jsonEditor.renderRoot.querySelector('[data-paramId]'); + + await renderHoveredElement(param); + + const showDefaultValuesButton = + jsonEditor.renderRoot.querySelector('devtools-button[title="Add a parameter"]'); + if (!showDefaultValuesButton) { + throw new Error('No button'); + } + + dispatchClickEvent(showDefaultValuesButton, { + bubbles: true, + composed: true, + }); + + await jsonEditor.updateComplete; + + // The -1 is need to not take into account the input for the command + const numberOfInputs = jsonEditor.renderRoot.querySelectorAll('devtools-suggestion-input').length - 1; + + assert.deepStrictEqual(numberOfInputs, 2); + }); + }); describe('Reset to default values', () => { @@ -703,6 +770,62 @@ assert.deepStrictEqual(value, []); }); + + it('should reset the value of optional object parameter to undefined after clicking on clear button', async () => { + const command = 'Test.test12'; + const typesByName = new Map(); + typesByName.set('Optional.Object', [ + { + 'optional': false, + 'type': 'string', + 'name': 'param1', + }, + { + 'optional': false, + 'type': 'number', + 'name': 'param2', + }, + ]); + const jsonEditor = renderJSONEditor(); + jsonEditor.typesByName = typesByName; + await populateMetadata(jsonEditor); + jsonEditor.command = command; + jsonEditor.populateParametersForCommandWithDefaultValues(); + await jsonEditor.updateComplete; + + const param = jsonEditor.renderRoot.querySelector('[data-paramId]'); + await renderHoveredElement(param); + + const showDefaultValuesButton = jsonEditor.renderRoot.querySelector('devtools-button[title="Add a parameter"]'); + if (!showDefaultValuesButton) { + throw new Error('No button'); + } + + dispatchClickEvent(showDefaultValuesButton, { + bubbles: true, + composed: true, + }); + + await jsonEditor.updateComplete; + + await renderHoveredElement(param); + const clearButton = jsonEditor.renderRoot.querySelector('devtools-button[title="Reset to default value"]'); + + if (!clearButton) { + throw new Error('No clear button'); + } + + dispatchClickEvent(clearButton, { + bubbles: true, + composed: true, + }); + + await jsonEditor.updateComplete; + // The -1 is need to not take into account the input for the command + const numberOfInputs = jsonEditor.renderRoot.querySelectorAll('devtools-suggestion-input').length - 1; + + assert.deepStrictEqual(numberOfInputs, 0); + }); }); describe('Delete and add for array parameters', () => {