From e20ec5777920f5558dc47cdf6bc15a4b0d65fcac Mon Sep 17 00:00:00 2001 From: "Cynthia S. Garcia" Date: Tue, 23 Mar 2021 10:31:55 +0100 Subject: [PATCH] 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 --- .../ChartTemplatesModal.module.css | 1 + .../chartTemplates/Template.module.css | 8 +++ .../package/chartTemplates/Template.tsx | 25 +++++++-- .../chartTemplates/__fixtures__/index/8.json | 3 ++ .../__snapshots__/Template.test.tsx.snap | 54 ++++++++++++++++--- .../__snapshots__/index.test.tsx.snap | 36 +++++++------ .../package/chartTemplates/index.test.tsx | 24 +++++++++ .../layout/package/chartTemplates/index.tsx | 53 +++++++++++------- web/src/layout/package/index.tsx | 1 + web/src/types.ts | 1 + web/src/utils/processHelmTemplate.tsx | 2 + 11 files changed, 162 insertions(+), 46 deletions(-) create mode 100644 web/src/layout/package/chartTemplates/__fixtures__/index/8.json diff --git a/web/src/layout/package/chartTemplates/ChartTemplatesModal.module.css b/web/src/layout/package/chartTemplates/ChartTemplatesModal.module.css index 046e042f..f37b0534 100644 --- 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 { diff --git a/web/src/layout/package/chartTemplates/Template.module.css b/web/src/layout/package/chartTemplates/Template.module.css index e27e8542..8ba41f39 100644 --- 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; } diff --git a/web/src/layout/package/chartTemplates/Template.tsx b/web/src/layout/package/chartTemplates/Template.tsx index 3ea55abb..56d7f98f 100644 --- 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(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 ( {parts.map((word: string, idx: number) => { + if (word === ')' || word === '|' || word.startsWith(`"`)) + return ( + + + {word} + {' '} + + ); return ( {regexifyString({ @@ -175,7 +192,7 @@ const Template = (props: Props) => { decorator: (match, index) => { return ( - {processHelmTemplateContent(match, index)} + {tokenizeContent(match, index)} ); }, diff --git a/web/src/layout/package/chartTemplates/__fixtures__/index/8.json b/web/src/layout/package/chartTemplates/__fixtures__/index/8.json new file mode 100644 index 00000000..11211316 --- /dev/null +++ b/web/src/layout/package/chartTemplates/__fixtures__/index/8.json @@ -0,0 +1,3 @@ +{ + "values": {} +} diff --git a/web/src/layout/package/chartTemplates/__snapshots__/Template.test.tsx.snap b/web/src/layout/package/chartTemplates/__snapshots__/Template.test.tsx.snap index b533588c..c1a15383 100644 --- 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 - "chart.resourceNamePrefix" . }} + + + "chart.resourceNamePrefix" + + . }} hub @@ -221,7 +227,19 @@ exports[`Template creates snapshot 1`] = ` > include - "chart.labels" . | + + + "chart.labels" + + . + + | + +
@@ -431,7 +449,13 @@ exports[`Template creates snapshot 1`] = ` > toYaml - . | + . + + | + +
@@ -604,7 +628,13 @@ exports[`Template creates snapshot 1`] = ` > include - "chart.resourceNamePrefix" . }} + + + "chart.resourceNamePrefix" + + . }} hub @@ -792,7 +822,13 @@ exports[`Template creates snapshot 1`] = ` > toYaml - . | + . + + | + +
@@ -1021,7 +1057,13 @@ exports[`Template creates snapshot 1`] = ` > toYaml - . | + . + + | + +
diff --git a/web/src/layout/package/chartTemplates/__snapshots__/index.test.tsx.snap b/web/src/layout/package/chartTemplates/__snapshots__/index.test.tsx.snap index 48ea9f4b..4f6f7aa4 100644 --- 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`] = `
- +
+ +
`; diff --git a/web/src/layout/package/chartTemplates/index.test.tsx b/web/src/layout/package/chartTemplates/index.test.tsx index 1a766389..5910621e 100644 --- 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( + + + + ); + + 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); diff --git a/web/src/layout/package/chartTemplates/index.tsx b/web/src/layout/package/chartTemplates/index.tsx index 9a59224c..16e38081 100644 --- 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 (
- +
+ +
{openStatus && templates && ( {
{ 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)) {