Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
14 changes: 8 additions & 6 deletions app/assets/javascripts/esm/enhanced-textbox.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -77,9 +77,11 @@ class EnhancedTextbox {

contentReplaced () {
return this.contentEscaped().replace(
this.tagPattern, (match, name, separator, value) => {
if (value && separator) {
return `<span class='placeholder-conditional'>((${name}??</span>${value}))`;
this.tagPattern, (match, name, separator, _, __, value) => {
if (value && separator == "??") {
return `<span class='placeholder-conditional'>((${name}${separator}</span>${value}))`;
} else if (value && separator == "::") {
return `<span class='placeholder-unsafe'>((${name}</span>${separator}${value}))`;
} else {
return `<span class='placeholder'>((${name}${value}))</span>`;
}
Expand Down
13 changes: 13 additions & 0 deletions app/assets/stylesheets/components/placeholder.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@

}

.placeholder-unsafe {

@extend %placeholder;
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 & {
Copy link
Contributor

@kr8n3r kr8n3r May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when in preview, the entire placeholder with ::unsafe is highlighted. intentional?

Screenshot 2025-05-01 at 07 51 48

seems due to in not being split up like in in the the textbox, as markup is
<span class="placeholder">((message::unsafe))</span>

all the content there comes from the utils
so i think if we want consistency then placeholder markup splitting might need to be done in utils

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've made those changes in utils but haven't brought them in yet. Happy to hold off deploying until they're ready to merge too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, i guess bringing it that version of utils in this PR would make sense, so you can check that both the placeholder styling in the message wrapper (above) looks ok as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can move this into draft until that's ready.

Pinned to the latest commit hash of the utils change so you can see what it's doing though

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;
Expand Down
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements_for_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements_for_test_common.in
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion ruff.toml
Original file line number Diff line number Diff line change
@@ -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/",
Expand Down
20 changes: 19 additions & 1 deletion tests/javascripts/enhance-textbox.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe('Enhanced textbox', () => {
<label class="govuk-label" for="template_content">Message</label>
<textarea class="govuk-textarea govuk-!-width-full govuk-textarea-highlight__textbox" id="template_content" name="template_content" rows="8" data-notify-module="enhanced-textbox" data-autofocus-textbox="true"></textarea>
</div>`;

textarea = document.querySelector('textarea');

new EnhancedTextbox(textarea);
Expand Down Expand Up @@ -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
Expand Down