Improve templates explorer modal (#1200)

- Do not highlight keywords in strings - Prevent page navigation when scrolling horizontally in templates - Fix issue cleaning up no templates available warning Signed-off-by: Cintia Sanchez Garcia <cynthiasg@icloud.com>
2021-03-23 10:31:55 +01:00 · 2021-03-23 10:31:55 +01:00 · e20ec57779
parent 5e73ab2e25
commit e20ec57779
11 changed files with 162 additions and 46 deletions
--- a/web/src/layout/package/chartTemplates/ChartTemplatesModal.module.css
+++ b/web/src/layout/package/chartTemplates/ChartTemplatesModal.module.css
@ -11,6 +11,7 @@
 .pre {
  opacity: 0.9 !important;
  font-size: 80%;
+  overscroll-behavior: contain;
 }

 .errorAlert {
--- a/web/src/layout/package/chartTemplates/Template.module.css
+++ b/web/src/layout/package/chartTemplates/Template.module.css
@ -20,6 +20,14 @@
  margin: 0 2px;
 }

+.specialCharacter {
+  margin-left: -6px;
+}
+
+.prevSpace::before {
+  content: ' ';
+}
+
 .tmplBuiltIn {
  color: #81a2be;
 }
--- a/web/src/layout/package/chartTemplates/Template.tsx
+++ b/web/src/layout/package/chartTemplates/Template.tsx
@ -1,4 +1,5 @@
-import { get, isEmpty, isObject, isString } from 'lodash';
+import classnames from 'classnames';
+import { get, isEmpty, isNull, isObject, isString } from 'lodash';
 import React, { useEffect, useState } from 'react';
 import regexifyString from 'regexify-string';

@ -18,6 +19,7 @@ const HIGHLIGHT_PATTERN = /{{(?!\/\*)(.*?)([^{]|{})*}}/;
 const FUNCTIONS_DEFINITIONS = require('./functions.json');
 const BUILTIN_DEFINITIONS = require('./builtIn.json');
 const SPECIAL_CHARACTERS = /[^|({})-]+/;
+const TOKENIZE_RE = /[^\s"']+|"([^"]*)"|'([^']*)/g;

 const Template = (props: Props) => {
  const [activeTemplate, setActiveTemplate] = useState<ChartTemplate>(props.template);
@ -128,11 +130,26 @@ const Template = (props: Props) => {
    }
  };

-  const processHelmTemplateContent = (str: string, lineNumber: number): JSX.Element => {
-    const parts = str.split(' ');
+  const tokenizeContent = (str: string, lineNumber: number): JSX.Element | null => {
+    const parts = str.match(TOKENIZE_RE);
+    if (isNull(parts)) return null;
    return (
      <span className={`badge font-weight-normal ${styles.badge}`}>
        {parts.map((word: string, idx: number) => {
+          if (word === ')' || word === '|' || word.startsWith(`"`))
+            return (
+              <React.Fragment key={`helmTmpl_${lineNumber}_${idx}`}>
+                <span
+                  className={classnames(
+                    'd-inline-flex',
+                    { [styles.specialCharacter]: word === ')' },
+                    { [styles.prevSpace]: word !== ')' }
+                  )}
+                >
+                  {word}
+                </span>{' '}
+              </React.Fragment>
+            );
          return (
            <React.Fragment key={`helmTmpl_${lineNumber}_${idx}`}>
              {regexifyString({
@ -175,7 +192,7 @@ const Template = (props: Props) => {
      decorator: (match, index) => {
        return (
          <span data-testid="betweenBracketsContent" key={`line_${index}`}>
-            {processHelmTemplateContent(match, index)}
+            {tokenizeContent(match, index)}
          </span>
        );
      },
--- a/web/src/layout/package/chartTemplates/fixtures/index/8.json
+++ b/web/src/layout/package/chartTemplates/fixtures/index/8.json
@ -0,0 +1,3 @@
+{
+  "values": {}
+}
--- a/web/src/layout/package/chartTemplates/snapshots/Template.test.tsx.snap
+++ b/web/src/layout/package/chartTemplates/snapshots/Template.test.tsx.snap
@ -154,7 +154,13 @@ exports[`Template creates snapshot 1`] = `
                >
                  include
                </span>
-                 "chart.resourceNamePrefix" . }}
+                 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  "chart.resourceNamePrefix"
+                </span>
+                 . }}
              </span>
            </span>
            hub
@ -221,7 +227,19 @@ exports[`Template creates snapshot 1`] = `
                >
                  include
                </span>
-                 "chart.labels" . | 
+                 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  "chart.labels"
+                </span>
+                 . 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  |
+                </span>
+                 
                <div
                  class="position-relative d-inline-block undefined"
                >
@ -431,7 +449,13 @@ exports[`Template creates snapshot 1`] = `
                >
                  toYaml
                </span>
-                 . | 
+                 . 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  |
+                </span>
+                 
                <div
                  class="position-relative d-inline-block undefined"
                >
@ -604,7 +628,13 @@ exports[`Template creates snapshot 1`] = `
                >
                  include
                </span>
-                 "chart.resourceNamePrefix" . }}
+                 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  "chart.resourceNamePrefix"
+                </span>
+                 . }}
              </span>
            </span>
            hub
@ -792,7 +822,13 @@ exports[`Template creates snapshot 1`] = `
                >
                  toYaml
                </span>
-                 . | 
+                 . 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  |
+                </span>
+                 
                <div
                  class="position-relative d-inline-block undefined"
                >
@ -1021,7 +1057,13 @@ exports[`Template creates snapshot 1`] = `
                >
                  toYaml
                </span>
-                 . | 
+                 . 
+                <span
+                  class="d-inline-flex prevSpace"
+                >
+                  |
+                </span>
+                 
                <div
                  class="position-relative d-inline-block undefined"
                >
--- a/web/src/layout/package/chartTemplates/snapshots/index.test.tsx.snap
+++ b/web/src/layout/package/chartTemplates/snapshots/index.test.tsx.snap
@ -5,25 +5,29 @@ exports[`ChartTemplatesModal creates snapshot 1`] = `
  <div
    class="mb-2"
  >
-    <button
-      class="btn btn-secondary btn-block btn-sm text-nowrap"
-      data-testid="tmplModalBtn"
+    <div
+      class="text-center"
    >
-      <div
-        class="d-flex flex-row align-items-center justify-content-center"
+      <button
+        class="btn btn-secondary btn-sm text-nowrap undefined"
+        data-testid="tmplModalBtn"
      >
-        <span
-          aria-hidden="true"
-          class="spinner-grow spinner-grow-sm"
-          role="status"
-        />
-        <span
-          class="d-none d-md-inline ml-2 font-weight-bold"
+        <div
+          class="d-flex flex-row align-items-center justify-content-center"
        >
-          Loading templates...
-        </span>
-      </div>
-    </button>
+          <span
+            aria-hidden="true"
+            class="spinner-grow spinner-grow-sm"
+            role="status"
+          />
+          <span
+            class="d-none d-md-inline ml-2 font-weight-bold"
+          >
+            Loading templates...
+          </span>
+        </div>
+      </button>
+    </div>
  </div>
 </DocumentFragment>
 `;
--- a/web/src/layout/package/chartTemplates/index.test.tsx
+++ b/web/src/layout/package/chartTemplates/index.test.tsx
@ -123,6 +123,30 @@ describe('ChartTemplatesModal', () => {
      });
    });

+    it('cleans url when templates list is empty', async () => {
+      const mockChartTemplates = getMockChartTemplates('8');
+      mocked(API).getChartTemplates.mockResolvedValue(mockChartTemplates);
+
+      render(
+        <Router>
+          <ChartTemplatesModal {...defaultProps} visibleChartTemplates />
+        </Router>
+      );
+
+      await waitFor(() => {
+        expect(API.getChartTemplates).toHaveBeenCalledTimes(1);
+        expect(API.getChartTemplates).toHaveBeenCalledWith('id', '1.1.1');
+        expect(mockHistoryReplace).toHaveBeenCalledTimes(1);
+        expect(mockHistoryReplace).toHaveBeenCalledWith({
+          search: '',
+          state: {
+            fromStarredPage: undefined,
+            searchUrlReferer: undefined,
+          },
+        });
+      });
+    });
+
    it('does not call again to getChartTemplates to open modal when package is the same', async () => {
      const mockChartTemplates = getMockChartTemplates('5');
      mocked(API).getChartTemplates.mockResolvedValue(mockChartTemplates);
--- a/web/src/layout/package/chartTemplates/index.tsx
+++ b/web/src/layout/package/chartTemplates/index.tsx
@ -24,6 +24,7 @@ interface Props {
  visibleTemplate?: string;
  searchUrlReferer?: SearchFiltersURL;
  fromStarredPage?: boolean;
+  btnClassName?: string;
 }

 interface FileProps {
@ -120,6 +121,13 @@ const ChartTemplatesModal = (props: Props) => {
    });
  };

+  const cleanUrl = () => {
+    history.replace({
+      search: '',
+      state: { searchUrlReferer: props.searchUrlReferer, fromStarredPage: props.fromStarredPage },
+    });
+  };
+
  useEffect(() => {
    if (props.visibleChartTemplates && !openStatus && (isUndefined(props.private) || !props.private)) {
      onOpenModal();
@ -157,6 +165,7 @@ const ChartTemplatesModal = (props: Props) => {
            type: 'warning',
            message: 'This Helm chart does not contain any template.',
          });
+          cleanUrl();
        }
      } else {
        setTemplates(null);
@ -164,6 +173,7 @@ const ChartTemplatesModal = (props: Props) => {
          type: 'warning',
          message: 'This Helm chart does not contain any template.',
        });
+        cleanUrl();
      }
      setIsLoading(false);
    } catch {
@ -173,6 +183,7 @@ const ChartTemplatesModal = (props: Props) => {
        type: 'danger',
        message: 'An error occurred getting chart templates, please try again later.',
      });
+      cleanUrl();
      setIsLoading(false);
    }
  }
@ -196,26 +207,28 @@ const ChartTemplatesModal = (props: Props) => {

  return (
    <div className="mb-2">
-      <button
-        data-testid="tmplModalBtn"
-        className="btn btn-secondary btn-block btn-sm text-nowrap"
-        onClick={onOpenModal}
-        disabled={!isUndefined(props.private) && props.private}
-      >
-        <div className="d-flex flex-row align-items-center justify-content-center">
-          {isLoading ? (
-            <>
-              <span className="spinner-grow spinner-grow-sm" role="status" aria-hidden="true" />
-              <span className="d-none d-md-inline ml-2 font-weight-bold">Loading templates...</span>
-            </>
-          ) : (
-            <>
-              <ImInsertTemplate />
-              <span className="ml-2 font-weight-bold text-uppercase">Templates</span>
-            </>
-          )}
-        </div>
-      </button>
+      <div className="text-center">
+        <button
+          data-testid="tmplModalBtn"
+          className={`btn btn-secondary btn-sm text-nowrap ${props.btnClassName}`}
+          onClick={onOpenModal}
+          disabled={!isUndefined(props.private) && props.private}
+        >
+          <div className="d-flex flex-row align-items-center justify-content-center">
+            {isLoading ? (
+              <>
+                <span className="spinner-grow spinner-grow-sm" role="status" aria-hidden="true" />
+                <span className="d-none d-md-inline ml-2 font-weight-bold">Loading templates...</span>
+              </>
+            ) : (
+              <>
+                <ImInsertTemplate />
+                <span className="ml-2 font-weight-bold text-uppercase">Templates</span>
+              </>
+            )}
+          </div>
+        </button>
+      </div>

      {openStatus && templates && (
        <Modal
--- a/web/src/layout/package/index.tsx
+++ b/web/src/layout/package/index.tsx
@ -545,6 +545,7 @@ const PackageView = (props: Props) => {

                          <div className="d-none d-lg-block">
                            <ChartTemplatesModal
+                              btnClassName="btn-block"
                              packageId={detail.packageId}
                              version={detail.version!}
                              repoKind={detail.repository.kind}
--- a/web/src/types.ts
+++ b/web/src/types.ts
@ -597,6 +597,7 @@ export enum ChartTemplateSpecialType {
  Function,
  FlowControl,
  Variable,
+  String,
 }

 export interface AHStats {
--- a/web/src/utils/processHelmTemplate.tsx
+++ b/web/src/utils/processHelmTemplate.tsx
@ -213,6 +213,8 @@ export const isBuiltInObject = (code: string): boolean => {
 export default (code: string): ChartTemplateSpecialType | null => {
  if (code.startsWith('.Values')) {
    return ChartTemplateSpecialType.ValuesBuiltInObject;
+  } else if (code.startsWith('"')) {
+    return ChartTemplateSpecialType.String;
  } else if (code.startsWith('$')) {
    return ChartTemplateSpecialType.Variable;
  } else if (FUNCTIONS.includes(code)) {