Solve cntrl+f issue on KVv2 JSON details and edit views (#28808)

* initial changes need to add test coverage

* change icon

* replace original idea with hds::codeblock on kvv2 details view

* changelog

* fixing edit view by addressing viewportMargin

* fix failing test

* missedone

* Update 28808.txt

* Update json-editor.js

* test coverage

* update codeblock selector

* Update general-selectors.ts

* Update kv-data-fields.js

* Update ui/lib/core/addon/components/json-editor.js

Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com>

* Update ui/lib/kv/addon/components/kv-data-fields.js

Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com>

* update test name

* add default to modifier

---------

Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com>

changelog/28808.txt

@@ -0,0 +1,6 @@
+```release-note:improvement
+ui: Replace KVv2 json secret details view with Hds::CodeBlock component allowing users to search the full secret height.
+```
+```release-note:bug
+ui: Allow users to search the full json object within the json code-editor edit/create view.
+```

ui/app/templates/components/console/log-json.hbs

@@ -8,7 +8,8 @@
     @showToolbar={{false}}
     @value={{stringify this.content}}
     @readOnly={{true}}
-    @viewportMargin="Infinity"
+    {{! ideally we calculate the "height" of the json data, but 100 should cover most cases }}
+    @viewportMargin="100"
     @gutters={{false}}
     @theme="hashi auto-height"
   />

ui/app/templates/components/control-group-success.hbs

@@ -21,7 +21,8 @@
           @showToolbar={{false}}
           @value={{stringify this.unwrapData}}
           @readOnly={{true}}
-          @viewportMargin="Infinity"
+          {{! ideally we calculate the "height" of the json data, but 100 should cover most cases }}
+          @viewportMargin="100"
           @gutters={{false}}
           @theme="hashi-read-only auto-height"
         />

ui/lib/core/addon/components/json-editor.hbs

@@ -51,7 +51,7 @@
       mode=@mode
       readOnly=@readOnly
       theme=@theme
-      viewportMarg=@viewportMargin
+      viewportMargin=@viewportMargin
       onSetup=this.onSetup
       onUpdate=this.onUpdate
       onFocus=this.onFocus

ui/lib/core/addon/components/json-editor.js

@@ -24,7 +24,7 @@ import { action } from '@ember/object';
  * @param {Boolean} [readOnly] - Sets the view to readOnly, allowing for copying but no editing. It also hides the cursor. Defaults to false.
  * @param {String} [theme] - Specify or customize the look via a named "theme" class in scss.
  * @param {String} [value] - Value within the display. Generally, a json string.
- * @param {String} [viewportMargin] - Size of viewport. Often set to "Infinity" to load/show all text regardless of length.
+ * @param {String} [viewportMargin] - Specifies the amount of lines rendered on the DOM (this is not the editor display height). The codemirror default is 10 which we set explicity in the code-mirror modifier per the recommendations from the codemirror docs.
  * @param {string} [example] - Example to show when value is null -- when example is provided a restore action will render in the toolbar to clear the current value and show the example after input
  * @param {string} [screenReaderLabel] - This label is read by the screen readers when CodeMirror text area is focused. This is helpful for accessibility.
  * @param {string} [container] - **REQUIRED if rendering within a modal** Selector string or element object of containing element, set the focused element as the container value. This is for the Hds::Copy::Button and to set `autoRefresh=true` so content renders https://hds-website-hashicorp.vercel.app/components/copy/button?tab=code

ui/lib/core/addon/modifiers/code-mirror.js

@@ -76,7 +76,7 @@ export default class CodeMirrorModifier extends Modifier {
       readOnly: namedArgs.readOnly || false,
       theme: namedArgs.theme || 'hashi',
       value: namedArgs.content || '',
-      viewportMargin: namedArgs.viewportMargin || '',
+      viewportMargin: namedArgs.viewportMargin || 10,
       autoRefresh: namedArgs.autoRefresh,
     });
 

ui/lib/kv/addon/components/kv-data-fields.hbs

@@ -13,12 +13,27 @@
 
 <hr class="is-marginless has-background-gray-200" />
 {{#if @showJson}}
-  <JsonEditor
-    @title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
-    @value={{this.stringifiedSecretData}}
-    @valueUpdated={{this.handleJson}}
-    @readOnly={{eq @type "details"}}
-  />
+  {{#if (eq @type "details")}}
+    <Hds::CodeBlock
+      data-test-code-block="secret-data"
+      @value={{this.stringifiedSecretData}}
+      @language="json"
+      @hasCopyButton={{true}}
+      @maxHeight="300px"
+      as |CB|
+    >
+      <CB.Title @tag="h3">
+        Version data
+      </CB.Title>
+    </Hds::CodeBlock>
+  {{else}}
+    <JsonEditor
+      @title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
+      @value={{this.stringifiedSecretData}}
+      @valueUpdated={{this.handleJson}}
+      @viewportMargin={{this.viewportMargin}}
+    />
+  {{/if}}
   {{#if (or @modelValidations.secretData.errors this.lintingErrors)}}
     <AlertInline
       @color={{if this.lintingErrors "warning" "critical"}}

ui/lib/kv/addon/components/kv-data-fields.js

@@ -41,6 +41,15 @@ export default class KvDataFields extends Component {
     return this.args.secret?.secretData ? stringify([this.args.secret.secretData], {}) : this.startingValue;
   }
 
+  get viewportMargin() {
+    if (!this.args?.secret?.secretData) return 10;
+    const jsonHeight = Object.keys(this.args.secret.secretData).length;
+    // return the higher of: 10 or the approimated number of lines in the json. jsonHeight only includes the first level of keys, so for objects
+    // with lots of nested values, it will undercount.
+    const max = Math.max(jsonHeight, 10);
+    return Math.min(max, 1000); // cap at 1000 lines to avoid performance implications
+  }
+
   @action
   handleJson(value, codemirror) {
     codemirror.performLint();

ui/lib/kv/addon/components/kv-patch/subkeys-reveal.hbs

@@ -8,6 +8,6 @@
     <Hds::Text::Body @tag="p" @weight="semibold">Reveal subkeys in JSON</Hds::Text::Body>
   </Toggle>
   {{#if this.showSubkeys}}
-    <Hds::CodeBlock @value={{stringify @subkeys}} @hasLineNumbers={{false}} data-test-subkeys />
+    <Hds::CodeBlock @value={{stringify @subkeys}} @hasLineNumbers={{false}} data-test-code-block="subkeys" />
   {{/if}}
 </div>

ui/lib/kv/addon/components/kv-paths-card.hbs

@@ -53,7 +53,7 @@
         for other CLI commands.
       </p>
       <Hds::CodeBlock
-        data-test-commands="cli"
+        data-test-code-block="cli"
         @language="bash"
         @hasLineNumbers={{false}}
         @hasCopyButton={{true}}
@@ -71,7 +71,7 @@
         </DocLink>
       </p>
       <Hds::CodeBlock
-        data-test-commands="api"
+        data-test-code-block="api"
         @language="bash"
         @hasLineNumbers={{false}}
         @hasCopyButton={{true}}

ui/lib/ldap/addon/components/page/library/details/accounts.hbs

@@ -55,7 +55,7 @@
       <:content>
         <Hds::CodeBlock
           class="has-top-margin-s"
-          data-test-accounts-code-block
+          data-test-code-block="accounts"
           @language="bash"
           @hasLineNumbers={{false}}
           @hasCopyButton={{true}}

ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js

@@ -299,22 +299,34 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
       codemirror().getValue(),
       `{
   \"\": \"\"
-}`
+}`,
+      'JSON editor displays correct empty object'
     );
     codemirror().setValue('{ "foo3": { "name": "bar3" } }');
     await click(FORM.saveBtn);
 
     // Details view
     await click(PAGE.secretTab('Secret'));
-    assert.dom(FORM.toggleJson).isNotDisabled();
-    assert.dom(FORM.toggleJson).isChecked();
-    assert.false(codemirror().getValue().includes('*'), 'Values are not obscured on details view');
+    assert.dom(FORM.toggleJson).isNotDisabled('JSON toggle is not disabled');
+    assert.dom(FORM.toggleJson).isChecked("JSON toggle is checked 'on'");
+
+    assert
+      .dom(GENERAL.codeBlock('secret-data'))
+      .hasText('Version data { "foo3": { "name": "bar3" } }', 'Values are displayed in the details view');
 
     // New version view
     await click(PAGE.detail.createNewVersion);
     assert.dom(FORM.toggleJson).isNotDisabled();
     assert.dom(FORM.toggleJson).isChecked();
-    assert.false(codemirror().getValue().includes('*'), 'Values are not obscured on edit view');
+    assert.deepEqual(
+      codemirror().getValue(),
+      `{
+  "foo3": {
+    "name": "bar3"
+  }
+}`,
+      'Values are displayed in the new version view'
+    );
   });
 
   test('on enter the JSON editor cursor goes to the next line', async function (assert) {
@@ -359,12 +371,16 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     await click(PAGE.secretTab('Secret'));
     await click(PAGE.detail.versionDropdown);
     await click(`${PAGE.detail.version(1)} a`);
-    assert.strictEqual(codemirror().getValue(), expectedDataV1, 'Version one data is displayed');
+    assert
+      .dom(GENERAL.codeBlock('secret-data'))
+      .hasText(`Version data ${expectedDataV1}`, 'Version one data is displayed');
 
     // Navigate back the second version and make sure the secret data is correct
     await click(PAGE.detail.versionDropdown);
     await click(`${PAGE.detail.version(2)} a`);
-    assert.strictEqual(codemirror().getValue(), expectedDataV2, 'Version two data is displayed');
+    assert
+      .dom(GENERAL.codeBlock('secret-data'))
+      .hasText(`Version data ${expectedDataV2}`, 'Version two data is displayed');
   });
 
   test('does not register as advanced when value includes {', async function (assert) {

ui/tests/helpers/general-selectors.ts

@@ -95,6 +95,7 @@ export const GENERAL = {
   cancelButton: '[data-test-cancel]',
   saveButton: '[data-test-save]',
   backButton: '[data-test-back-button]',
+  codeBlock: (label: string) => `[data-test-code-block="${label}"]`,
   codemirror: `[data-test-component="code-mirror-modifier"]`,
   codemirrorTextarea: `[data-test-component="code-mirror-modifier"] textarea`,
 };

ui/tests/helpers/kv/kv-selectors.js

@@ -90,8 +90,8 @@ export const PAGE = {
   },
   paths: {
     copyButton: (label) => `${PAGE.infoRowValue(label)} button`,
-    codeSnippet: (section) => `[data-test-commands="${section}"] code`,
-    snippetCopy: (section) => `[data-test-commands="${section}"] button`,
+    codeSnippet: (section) => `[data-test-code-block="${section}"] code`,
+    snippetCopy: (section) => `[data-test-code-block="${section}"] button`,
   },
 };
 

ui/tests/helpers/secret-engine/secret-engine-helpers.js

@@ -206,3 +206,49 @@ export const fillInAwsConfig = async (situation = 'withAccess') => {
     await fillIn(GENERAL.ttl.input('Identity token TTL'), '7200');
   }
 };
+
+// Example usage
+// createLongJson (2, 3) will create a json object with 2 original keys, each with 3 nested keys
+// {
+// 	"key-0": {
+// 		"nested-key-0": {
+// 			"nested-key-1": {
+// 				"nested-key-2": "nested-value"
+// 			}
+// 		}
+// 	},
+// 	"key-1": {
+// 		"nested-key-0": {
+// 			"nested-key-1": {
+// 				"nested-key-2": "nested-value"
+// 			}
+// 		}
+// 	}
+// }
+
+export function createLongJson(lines = 10, nestLevel = 3) {
+  const keys = Array.from({ length: nestLevel }, (_, i) => `nested-key-${i}`);
+  const jsonObject = {};
+
+  for (let i = 0; i < lines; i++) {
+    nestLevel > 0
+      ? (jsonObject[`key-${i}`] = createNestedObject({}, keys, 'nested-value'))
+      : (jsonObject[`key-${i}`] = 'non-nested-value');
+  }
+  return jsonObject;
+}
+
+function createNestedObject(obj = {}, keys, value) {
+  let current = obj;
+
+  for (let i = 0; i < keys.length - 1; i++) {
+    const key = keys[i];
+    if (!current[key]) {
+      current[key] = {};
+    }
+    current = current[key];
+  }
+
+  current[keys[keys.length - 1]] = value;
+  return obj;
+}

ui/tests/integration/components/json-editor-test.js

@@ -11,6 +11,7 @@ import hbs from 'htmlbars-inline-precompile';
 import jsonEditor from '../../pages/components/json-editor';
 import sinon from 'sinon';
 import { setRunOptions } from 'ember-a11y-testing/test-support';
+import { createLongJson } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';
 
 const component = create(jsonEditor);
 
@@ -29,6 +30,7 @@ module('Integration | Component | json-editor', function (hooks) {
     this.set('onFocusOut', sinon.spy());
     this.set('json_blob', JSON_BLOB);
     this.set('bad_json_blob', BAD_JSON_BLOB);
+    this.set('long_json', JSON.stringify(createLongJson(), null, `\t`));
     this.set('hashi-read-only-theme', 'hashi-read-only auto-height');
     setRunOptions({
       rules: {
@@ -36,6 +38,8 @@ module('Integration | Component | json-editor', function (hooks) {
         label: { enabled: false },
         // TODO: investigate and fix Codemirror styling
         'color-contrast': { enabled: false },
+        // failing on .CodeMirror-scroll
+        'scrollable-region-focusable': { enabled: false },
       },
     });
   });
@@ -129,4 +133,31 @@ module('Integration | Component | json-editor', function (hooks) {
       'even after hitting enter the value is still set correctly'
     );
   });
+
+  test('no viewportMargin renders only default 10 lines of data on the DOM', async function (assert) {
+    await render(hbs`
+      <JsonEditor
+        @value={{this.long_json}}
+        @mode="ruby"
+        @valueUpdated={{fn (mut this.value)}}
+      />
+    `);
+    assert
+      .dom('.CodeMirror-code')
+      .doesNotIncludeText('key-9', 'Without viewportMargin, user cannot search for key-9');
+  });
+
+  test('when viewportMargin is set user is able to search a long secret', async function (assert) {
+    await render(hbs`
+      <JsonEditor
+        @value={{this.long_json}}
+        @mode="ruby"
+        @valueUpdated={{fn (mut this.value)}}
+        @viewportMargin="100"
+      />
+    `);
+    assert
+      .dom('.CodeMirror-code')
+      .containsText('key-9', 'With viewportMargin set, user can search for key-9');
+  });
 });

ui/tests/integration/components/kv/kv-data-fields-test.js

@@ -11,6 +11,9 @@ import { hbs } from 'ember-cli-htmlbars';
 import { fillIn, render, click } from '@ember/test-helpers';
 import codemirror from 'vault/tests/helpers/codemirror';
 import { PAGE, FORM } from 'vault/tests/helpers/kv/kv-selectors';
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
+import { setRunOptions } from 'ember-a11y-testing/test-support';
+import { createLongJson } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';
 
 module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
   setupRenderingTest(hooks);
@@ -22,6 +25,12 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
     this.backend = 'my-kv-engine';
     this.path = 'my-secret';
     this.secret = this.store.createRecord('kv/data', { backend: this.backend });
+    setRunOptions({
+      rules: {
+        // failing on .CodeMirror-scroll
+        'scrollable-region-focusable': { enabled: false },
+      },
+    });
   });
 
   test('it updates the secret model', async function (assert) {
@@ -88,7 +97,7 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
     assert.dom(PAGE.infoRowValue('foo')).hasText('bar', 'secret value shows after toggle');
   });
 
-  test('it shows readonly json editor when viewing secret details of complex secret', async function (assert) {
+  test('it shows hds codeblock when viewing secret details of complex secret', async function (assert) {
     this.secret.secretData = {
       foo: {
         bar: 'baz',
@@ -100,7 +109,24 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
       owner: this.engine,
     });
     assert.dom(PAGE.infoRowValue('foo')).doesNotExist('does not render rows of secret data');
-    assert.dom('[data-test-component="code-mirror-modifier"]').hasClass('readonly-codemirror');
-    assert.dom('[data-test-component="code-mirror-modifier"]').includesText(`{ "foo": { "bar": "baz" }}`);
+    assert.dom(GENERAL.codeBlock('secret-data')).exists('hds codeBlock exists');
+    assert
+      .dom(GENERAL.codeBlock('secret-data'))
+      .hasText(`Version data { "foo": { "bar": "baz" } } `, 'Json data is displayed');
+  });
+
+  test('it defaults to a viewportMargin 10 when there is no secret data', async function (assert) {
+    await render(hbs`<KvDataFields @showJson={{true}} @secret={{this.secret}} />`, { owner: this.engine });
+    assert.strictEqual(codemirror().options.viewportMargin, 10, 'viewportMargin defaults to 10');
+  });
+
+  test('it calculates viewportMargin based on secret size', async function (assert) {
+    this.secret.secretData = createLongJson(100);
+    await render(hbs`<KvDataFields @showJson={{true}} @secret={{this.secret}} />`, { owner: this.engine });
+    assert.strictEqual(
+      codemirror().options.viewportMargin,
+      100,
+      'viewportMargin is set to 100 matching the height of the json'
+    );
   });
 });

ui/tests/integration/components/kv/kv-patch/editor/form-test.js

@@ -96,14 +96,14 @@ module('Integration | Component | kv | kv-patch/editor/form', function (hooks) {
     await this.renderComponent();
 
     assert.dom(GENERAL.toggleInput('Reveal subkeys')).isNotChecked('toggle is initially unchecked');
-    assert.dom('[data-test-subkeys]').doesNotExist();
+    assert.dom(GENERAL.codeBlock('subkeys')).doesNotExist();
     await click(GENERAL.toggleInput('Reveal subkeys'));
     assert.dom(GENERAL.toggleInput('Reveal subkeys')).isChecked();
-    assert.dom('[data-test-subkeys]').hasText(JSON.stringify(this.subkeys, null, 2));
+    assert.dom(GENERAL.codeBlock('subkeys')).hasText(JSON.stringify(this.subkeys, null, 2));
 
     await click(GENERAL.toggleInput('Reveal subkeys'));
     assert.dom(GENERAL.toggleInput('Reveal subkeys')).isNotChecked();
-    assert.dom('[data-test-subkeys]').doesNotExist('unchecking re-hides subkeys');
+    assert.dom(GENERAL.codeBlock('subkeys')).doesNotExist('unchecking re-hides subkeys');
   });
 
   test('it enables and disables inputs', async function (assert) {

ui/tests/integration/components/kv/kv-patch/json-form-test.js

@@ -59,14 +59,14 @@ module('Integration | Component | kv | kv-patch/editor/json-form', function (hoo
     await this.renderComponent();
 
     assert.dom(GENERAL.toggleInput('Reveal subkeys')).isNotChecked('toggle is initially unchecked');
-    assert.dom('[data-test-subkeys]').doesNotExist();
+    assert.dom(GENERAL.codeBlock('subkeys')).doesNotExist();
     await click(GENERAL.toggleInput('Reveal subkeys'));
     assert.dom(GENERAL.toggleInput('Reveal subkeys')).isChecked();
-    assert.dom('[data-test-subkeys]').hasText(JSON.stringify(this.subkeys, null, 2));
+    assert.dom(GENERAL.codeBlock('subkeys')).hasText(JSON.stringify(this.subkeys, null, 2));
 
     await click(GENERAL.toggleInput('Reveal subkeys'));
     assert.dom(GENERAL.toggleInput('Reveal subkeys')).isNotChecked();
-    assert.dom('[data-test-subkeys]').doesNotExist('unchecking re-hides subkeys');
+    assert.dom(GENERAL.codeBlock('subkeys')).doesNotExist('unchecking re-hides subkeys');
   });
 
   test('it renders linting errors', async function (assert) {

ui/tests/integration/components/kv/page/kv-page-secret-details-test.js

@@ -7,13 +7,14 @@ import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import { setupEngine } from 'ember-engines/test-support';
 import { setupMirage } from 'ember-cli-mirage/test-support';
-import { click, find, render } from '@ember/test-helpers';
+import { click, render } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import { kvDataPath } from 'vault/utils/kv-path';
-import { FORM, PAGE, parseJsonEditor } from 'vault/tests/helpers/kv/kv-selectors';
+import { FORM, PAGE } from 'vault/tests/helpers/kv/kv-selectors';
 import { syncStatusResponse } from 'vault/mirage/handlers/sync';
 import { encodePath } from 'vault/utils/path-encoding-helpers';
 import { baseSetup } from 'vault/tests/helpers/kv/kv-run-commands';
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
 
 module('Integration | Component | kv-v2 | Page::Secret::Details', function (hooks) {
   setupRenderingTest(hooks);
@@ -126,19 +127,24 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
     await click(FORM.toggleMasked);
     assert.dom(PAGE.infoRowValue('foo')).hasText('bar', 'renders secret value');
     await click(FORM.toggleJson);
-    assert.propEqual(parseJsonEditor(find), this.secretData, 'json editor renders secret data');
+    assert.dom(GENERAL.codeBlock('secret-data')).hasText(
+      `Version data {
+  "foo": "bar"
+}`,
+      'json editor renders secret data'
+    );
     assert
       .dom(PAGE.detail.versionTimestamp)
       .includesText(`Version ${this.version} created`, 'renders version and time created');
   });
 
-  test('it renders json view when secret is complex', async function (assert) {
+  test('it renders hds codeblock view when secret is complex', async function (assert) {
     assert.expect(4);
     await this.renderComponent(this.modelComplex);
     assert.dom(PAGE.infoRowValue('foo')).doesNotExist('does not render rows of secret data');
     assert.dom(FORM.toggleJson).isChecked();
     assert.dom(FORM.toggleJson).isNotDisabled();
-    assert.dom('[data-test-component="code-mirror-modifier"]').exists('shows json editor');
+    assert.dom(GENERAL.codeBlock('secret-data')).exists('hds codeBlock exists');
   });
 
   test('it renders deleted empty state', async function (assert) {

ui/tests/integration/components/ldap/page/library/details/accounts-test.js

@@ -11,6 +11,7 @@ import { render, click, fillIn } from '@ember/test-helpers';
 import hbs from 'htmlbars-inline-precompile';
 import { allowAllCapabilitiesStub } from 'vault/tests/helpers/stubs';
 import sinon from 'sinon';
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
 
 module('Integration | Component | ldap | Page::Library::Details::Accounts', function (hooks) {
   setupRenderingTest(hooks);
@@ -76,7 +77,7 @@ module('Integration | Component | ldap | Page::Library::Details::Accounts', func
     assert.dom('[data-test-checked-out-card]').exists('Accounts checked out card renders');
 
     assert
-      .dom('[data-test-accounts-code-block] code')
+      .dom(`${GENERAL.codeBlock('accounts')} code`)
       .hasText(
         'vault lease renew ldap-test/library/test-library/check-out/:lease_id',
         'Renew cli command renders with backend path'