From a09a1a70c95d27587600769f6089f6c7c4b65e93 Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Wed, 30 Apr 2025 16:15:45 +0100 Subject: [PATCH 1/6] Added new style for unsafe placeholders The normal placeholder css class includes a box shadow that accounts for trailing parentheses so that they do not get highlighted. This is the wrong behaviour for unsafe placeholders because it would result in the last character of a placeholders variable name not being highlighted --- .../stylesheets/components/placeholder.scss | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/assets/stylesheets/components/placeholder.scss b/app/assets/stylesheets/components/placeholder.scss index 833bb9d760..a3e66b4fe1 100644 --- a/app/assets/stylesheets/components/placeholder.scss +++ b/app/assets/stylesheets/components/placeholder.scss @@ -14,6 +14,22 @@ } +.placeholder-unsafe { + + @extend %placeholder; + border-radius: 0; + border-top-left-radius: 20px; + border-bottom-left-radius: 20px; + border-top-right-radius: 8px; + border-bottom-right-radius: 8px; + box-shadow: inset 0.47em 0 0 0 #FFF, inset 0 -0.05em 0 0 #FFF, inset 0 0.26em 0 0 #FFF; + + .sms-message-wrapper & { + box-shadow: inset 0.47em 0 0 0 govuk-shade(govuk-colour("light-grey"), 7%), inset -0.89em 0 0 0 govuk-shade(govuk-colour("light-grey"), 7%), inset 0 -0.18em 0 0 govuk-shade(govuk-colour("light-grey"), 7%), inset 0 0.18em 0 0 govuk-shade(govuk-colour("light-grey"), 7%); + } + +} + .placeholder-no-brackets { @extend %placeholder; padding-left: 3px; From e0fd110c8e3b947e77bed86ee5b0d07b09ca31e0 Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Wed, 30 Apr 2025 16:17:47 +0100 Subject: [PATCH 2/6] updated the enhanced-textbox module to properly account for unsafe placeholders when editing templates javascript should dynamically style text so that the background of a placeholders variable name is highlighted. This commit extends the regular expression to include the `::` delimeter of unsafe placeholders, let's us split the placeholder string up based on this capturing group so that the highlighted variable is styled properly --- .../javascripts/esm/enhanced-textbox.mjs | 14 +++++++------ tests/javascripts/enhance-textbox.test.mjs | 20 ++++++++++++++++++- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/esm/enhanced-textbox.mjs b/app/assets/javascripts/esm/enhanced-textbox.mjs index 2075bd1e7b..615ef9750b 100644 --- a/app/assets/javascripts/esm/enhanced-textbox.mjs +++ b/app/assets/javascripts/esm/enhanced-textbox.mjs @@ -17,7 +17,7 @@ class EnhancedTextbox { } this.$module = $module; - this.tagPattern = /\(\(([^\)\((\?)]+)(\?\?)?([^\)\(]*)\)\)/g; + this.tagPattern = /\(\(([^\)\((\?)(\:)]+)((\?\?)|(::))?([^\)\(]*)\)\)/g; this.highlightPlaceholders = this.$module.dataset.highlightPlaceholders === 'true'; this.autofocus = this.$module.dataset.autofocusTextbox === 'true'; this.$textbox = this.$module; @@ -42,10 +42,10 @@ class EnhancedTextbox { this.$visibleTextbox.style.visibility = 'hidden'; this.$visibleTextbox.style.display = 'block'; document.querySelector('body').append(this.$visibleTextbox); - + this.initialHeight = this.$visibleTextbox.offsetHeight; - this.$backgroundHighlightElement.style.borderWidth = + this.$backgroundHighlightElement.style.borderWidth = window.getComputedStyle(this.$textbox).getPropertyValue('border-width'); this.$visibleTextbox.remove(); @@ -77,9 +77,11 @@ class EnhancedTextbox { contentReplaced () { return this.contentEscaped().replace( - this.tagPattern, (match, name, separator, value) => { - if (value && separator) { - return `((${name}??${value}))`; + this.tagPattern, (match, name, separator, _, __, value) => { + if (value && separator == "??") { + return `((${name}${separator}${value}))`; + } else if (value && separator == "::") { + return `((${name}${separator}${value}))`; } else { return `((${name}${value}))`; } diff --git a/tests/javascripts/enhance-textbox.test.mjs b/tests/javascripts/enhance-textbox.test.mjs index e301c75e14..12d574f0d5 100644 --- a/tests/javascripts/enhance-textbox.test.mjs +++ b/tests/javascripts/enhance-textbox.test.mjs @@ -162,7 +162,7 @@ describe('Enhanced textbox', () => { `; - + textarea = document.querySelector('textarea'); new EnhancedTextbox(textarea); @@ -358,8 +358,26 @@ describe('Enhanced textbox', () => { }); + test("If an unsafe placeholder is added, it should be highlighted with placeholder-unsafe style", () => { + + textarea.textContent = "Dear ((title::unsafe)) ((surname::unsafe))"; + + // start module + new EnhancedTextbox(textarea); + + backgroundEl = textarea.nextElementSibling; + helpers.triggerEvent(textarea, 'input'); + + // add some more content with some optional content inside + const unsafeHighlightTags = backgroundEl.querySelectorAll('.placeholder-unsafe'); + + expect(unsafeHighlightTags.length).toEqual(2); + + }); + }); + describe("When the content of the textbox is updated", () => { // doesn't apply to inputs as they have a fixed height From 5ad8e8aaeb9ee21f99c70e22ac390702dae5ac98 Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Thu, 1 May 2025 09:24:20 +0100 Subject: [PATCH 3/6] remove unused css class --- app/assets/javascripts/esm/enhanced-textbox.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/esm/enhanced-textbox.mjs b/app/assets/javascripts/esm/enhanced-textbox.mjs index 615ef9750b..83383dc590 100644 --- a/app/assets/javascripts/esm/enhanced-textbox.mjs +++ b/app/assets/javascripts/esm/enhanced-textbox.mjs @@ -81,7 +81,7 @@ class EnhancedTextbox { if (value && separator == "??") { return `((${name}${separator}${value}))`; } else if (value && separator == "::") { - return `((${name}${separator}${value}))`; + return `((${name}${separator}${value}))`; } else { return `((${name}${value}))`; } From ff448910eee606fcd0fee85b77394451df90ad62 Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Thu, 1 May 2025 09:33:06 +0100 Subject: [PATCH 4/6] fixed border radius --- app/assets/stylesheets/components/placeholder.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/components/placeholder.scss b/app/assets/stylesheets/components/placeholder.scss index a3e66b4fe1..4431a01224 100644 --- a/app/assets/stylesheets/components/placeholder.scss +++ b/app/assets/stylesheets/components/placeholder.scss @@ -20,8 +20,8 @@ border-radius: 0; border-top-left-radius: 20px; border-bottom-left-radius: 20px; - border-top-right-radius: 8px; - border-bottom-right-radius: 8px; + border-top-right-radius: 0px; + border-bottom-right-radius: 0px; box-shadow: inset 0.47em 0 0 0 #FFF, inset 0 -0.05em 0 0 #FFF, inset 0 0.26em 0 0 #FFF; .sms-message-wrapper & { From 8a685585b6973ec80d99c680156a282a3442871f Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Thu, 1 May 2025 15:13:41 +0100 Subject: [PATCH 5/6] removed redundant styling from inherited class --- app/assets/stylesheets/components/placeholder.scss | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/assets/stylesheets/components/placeholder.scss b/app/assets/stylesheets/components/placeholder.scss index 4431a01224..dea82fb30b 100644 --- a/app/assets/stylesheets/components/placeholder.scss +++ b/app/assets/stylesheets/components/placeholder.scss @@ -17,9 +17,6 @@ .placeholder-unsafe { @extend %placeholder; - border-radius: 0; - border-top-left-radius: 20px; - border-bottom-left-radius: 20px; border-top-right-radius: 0px; border-bottom-right-radius: 0px; box-shadow: inset 0.47em 0 0 0 #FFF, inset 0 -0.05em 0 0 #FFF, inset 0 0.26em 0 0 #FFF; From 47f708217998d9dff1104b52e2a3bcd0ca48cf1d Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Fri, 16 May 2025 11:51:56 +0100 Subject: [PATCH 6/6] pin to utils feature branch for testing --- .pre-commit-config.yaml | 2 +- requirements.in | 2 +- requirements.txt | 2 +- requirements_for_test.txt | 2 +- requirements_for_test_common.in | 2 +- ruff.toml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c6472f0bde..a9687ce6c5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,4 @@ -# This file was automatically copied from notifications-utils@99.3.3 +# This file was automatically copied from notifications-utils@99.4.1 repos: - repo: https://github.com/pre-commit/pre-commit-hooks diff --git a/requirements.in b/requirements.in index ad765ccd3d..c49316aa4d 100644 --- a/requirements.in +++ b/requirements.in @@ -17,7 +17,7 @@ notifications-python-client==10.0.0 fido2==1.1.3 # Run `make bump-utils` to update to the latest version -notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@99.3.3 +notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@5d01731e30708245bb102a3b8d078e246fe8c531 govuk-frontend-jinja==3.5.0 diff --git a/requirements.txt b/requirements.txt index c76501439f..3f563509b9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -101,7 +101,7 @@ mistune==0.8.4 # via notifications-utils notifications-python-client==10.0.0 # via -r requirements.in -notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@7d7a531ec102078988e2cdc1493a56c2cc5d0368 +notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@5d01731e30708245bb102a3b8d078e246fe8c531 # via -r requirements.in openpyxl==3.1.5 # via pyexcel-xlsx diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 4367032d06..71e799cff2 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -164,7 +164,7 @@ moto==5.1.0 # via -r requirements_for_test.in notifications-python-client==10.0.0 # via -r requirements.txt -notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@7d7a531ec102078988e2cdc1493a56c2cc5d0368 +notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@5d01731e30708245bb102a3b8d078e246fe8c531 # via -r requirements.txt openpyxl==3.1.5 # via diff --git a/requirements_for_test_common.in b/requirements_for_test_common.in index d1ee4f3041..0e5aca7c1e 100644 --- a/requirements_for_test_common.in +++ b/requirements_for_test_common.in @@ -1,4 +1,4 @@ -# This file was automatically copied from notifications-utils@99.3.3 +# This file was automatically copied from notifications-utils@99.4.1 beautifulsoup4==4.12.3 pytest==8.3.4 diff --git a/ruff.toml b/ruff.toml index 16dbb867c7..c759059f0f 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,4 +1,4 @@ -# This file was automatically copied from notifications-utils@99.3.3 +# This file was automatically copied from notifications-utils@99.4.1 extend-exclude = [ "migrations/versions/",