-
Notifications
You must be signed in to change notification settings - Fork 347
Refactor Character count method to reduce repeated updates #6449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
JavaScript changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
index 61b98c577..a6d4b0a31 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -417,7 +417,7 @@ class CharacterCount extends ConfigurableComponent {
}
constructor(t, e = {}) {
var i, n;
- super(t, e), this.$textarea = void 0, this.$visibleCountMessage = void 0, this.$screenReaderCountMessage = void 0, this.lastInputTimestamp = null, this.lastInputValue = "", this.valueChecker = null, this.i18n = void 0, this.maxLength = void 0;
+ super(t, e), this.$textarea = void 0, this.count = 0, this.$visibleCountMessage = void 0, this.$screenReaderCountMessage = void 0, this.lastInputTimestamp = null, this.lastInputValue = "", this.valueChecker = null, this.i18n = void 0, this.maxLength = void 0;
const s = this.$root.querySelector(".govuk-js-character-count");
if (!(s instanceof HTMLTextAreaElement || s instanceof HTMLInputElement)) throw new ElementError({
component: CharacterCount,
@@ -457,13 +457,15 @@ class CharacterCount extends ConfigurableComponent {
const l = document.createElement("div");
l.className = "govuk-character-count__sr-status govuk-visually-hidden", l.setAttribute("aria-live", "polite"), this.$screenReaderCountMessage = l, a.insertAdjacentElement("afterend", l);
const c = document.createElement("div");
- c.className = a.className, c.classList.add("govuk-character-count__status"), c.setAttribute("aria-hidden", "true"), this.$visibleCountMessage = c, a.insertAdjacentElement("afterend", c), a.classList.add("govuk-visually-hidden"), this.$textarea.removeAttribute("maxlength"), this.bindChangeEvents(), window.addEventListener("pageshow", (() => this.updateCountMessage())), this.updateCountMessage()
+ c.className = a.className, c.classList.add("govuk-character-count__status"), c.setAttribute("aria-hidden", "true"), this.$visibleCountMessage = c, a.insertAdjacentElement("afterend", c), a.classList.add("govuk-visually-hidden"), this.$textarea.removeAttribute("maxlength"), this.bindChangeEvents(), window.addEventListener("pageshow", (() => {
+ this.$textarea.value !== this.$textarea.innerHTML && (this.updateCount(), this.updateCountMessage())
+ })), this.updateCount(), this.updateCountMessage()
}
bindChangeEvents() {
- this.$textarea.addEventListener("keyup", (() => this.handleKeyUp())), this.$textarea.addEventListener("focus", (() => this.handleFocus())), this.$textarea.addEventListener("blur", (() => this.handleBlur()))
+ this.$textarea.addEventListener("input", (() => this.handleInput())), this.$textarea.addEventListener("focus", (() => this.handleFocus())), this.$textarea.addEventListener("blur", (() => this.handleBlur()))
}
- handleKeyUp() {
- this.updateVisibleCountMessage(), this.lastInputTimestamp = Date.now()
+ handleInput() {
+ this.updateCount(), this.updateVisibleCountMessage(), this.lastInputTimestamp = Date.now()
}
handleFocus() {
this.valueChecker = window.setInterval((() => {
@@ -480,21 +482,22 @@ class CharacterCount extends ConfigurableComponent {
this.updateVisibleCountMessage(), this.updateScreenReaderCountMessage()
}
updateVisibleCountMessage() {
- const t = this.maxLength - this.count(this.$textarea.value) < 0;
+ const t = this.maxLength - this.count < 0;
this.$visibleCountMessage.classList.toggle("govuk-character-count__message--disabled", !this.isOverThreshold()), this.$errorMessage || this.$textarea.classList.toggle("govuk-textarea--error", t), this.$visibleCountMessage.classList.toggle("govuk-error-message", t), this.$visibleCountMessage.classList.toggle("govuk-hint", !t), this.$visibleCountMessage.textContent = this.getCountMessage()
}
updateScreenReaderCountMessage() {
this.isOverThreshold() ? this.$screenReaderCountMessage.removeAttribute("aria-hidden") : this.$screenReaderCountMessage.setAttribute("aria-hidden", "true"), this.$screenReaderCountMessage.textContent = this.getCountMessage()
}
- count(t) {
+ updateCount() {
+ const t = this.$textarea.value;
if (this.config.maxwords) {
var e;
- return (null != (e = t.match(/\S+/g)) ? e : []).length
- }
- return t.length
+ const i = null != (e = t.match(/\S+/g)) ? e : [];
+ this.count = i.length
+ } else this.count = t.length
}
getCountMessage() {
- const t = this.maxLength - this.count(this.$textarea.value),
+ const t = this.maxLength - this.count,
e = this.config.maxwords ? "words" : "characters";
return this.formatCountMessage(t, e)
}
@@ -507,7 +510,7 @@ class CharacterCount extends ConfigurableComponent {
}
isOverThreshold() {
if (!this.config.threshold) return !0;
- const t = this.count(this.$textarea.value);
+ const t = this.count;
return this.maxLength * this.config.threshold / 100 <= t
}
}
Action run for d0b52ec |
Other changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/all.bundle.js b/packages/govuk-frontend/dist/govuk/all.bundle.js
index a185518a3..85e06a380 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -886,6 +886,7 @@
var _ref, _this$config$maxwords;
super($root, config);
this.$textarea = void 0;
+ this.count = 0;
this.$visibleCountMessage = void 0;
this.$screenReaderCountMessage = void 0;
this.lastInputTimestamp = null;
@@ -941,15 +942,22 @@
$textareaDescription.classList.add('govuk-visually-hidden');
this.$textarea.removeAttribute('maxlength');
this.bindChangeEvents();
- window.addEventListener('pageshow', () => this.updateCountMessage());
+ window.addEventListener('pageshow', () => {
+ if (this.$textarea.value !== this.$textarea.innerHTML) {
+ this.updateCount();
+ this.updateCountMessage();
+ }
+ });
+ this.updateCount();
this.updateCountMessage();
}
bindChangeEvents() {
- this.$textarea.addEventListener('keyup', () => this.handleKeyUp());
+ this.$textarea.addEventListener('input', () => this.handleInput());
this.$textarea.addEventListener('focus', () => this.handleFocus());
this.$textarea.addEventListener('blur', () => this.handleBlur());
}
- handleKeyUp() {
+ handleInput() {
+ this.updateCount();
this.updateVisibleCountMessage();
this.lastInputTimestamp = Date.now();
}
@@ -976,7 +984,7 @@
this.updateScreenReaderCountMessage();
}
updateVisibleCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const isError = remainingNumber < 0;
this.$visibleCountMessage.classList.toggle('govuk-character-count__message--disabled', !this.isOverThreshold());
if (!this.$errorMessage) {
@@ -994,16 +1002,18 @@
}
this.$screenReaderCountMessage.textContent = this.getCountMessage();
}
- count(text) {
+ updateCount() {
+ const text = this.$textarea.value;
if (this.config.maxwords) {
var _text$match;
const tokens = (_text$match = text.match(/\S+/g)) != null ? _text$match : [];
- return tokens.length;
+ this.count = tokens.length;
+ return;
}
- return text.length;
+ this.count = text.length;
}
getCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const countType = this.config.maxwords ? 'words' : 'characters';
return this.formatCountMessage(remainingNumber, countType);
}
@@ -1020,7 +1030,7 @@
if (!this.config.threshold) {
return true;
}
- const currentLength = this.count(this.$textarea.value);
+ const currentLength = this.count;
const maxLength = this.maxLength;
const thresholdValue = maxLength * this.config.threshold / 100;
return thresholdValue <= currentLength;
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index e3bf09d22..170744084 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -880,6 +880,7 @@ class CharacterCount extends ConfigurableComponent {
var _ref, _this$config$maxwords;
super($root, config);
this.$textarea = void 0;
+ this.count = 0;
this.$visibleCountMessage = void 0;
this.$screenReaderCountMessage = void 0;
this.lastInputTimestamp = null;
@@ -935,15 +936,22 @@ class CharacterCount extends ConfigurableComponent {
$textareaDescription.classList.add('govuk-visually-hidden');
this.$textarea.removeAttribute('maxlength');
this.bindChangeEvents();
- window.addEventListener('pageshow', () => this.updateCountMessage());
+ window.addEventListener('pageshow', () => {
+ if (this.$textarea.value !== this.$textarea.innerHTML) {
+ this.updateCount();
+ this.updateCountMessage();
+ }
+ });
+ this.updateCount();
this.updateCountMessage();
}
bindChangeEvents() {
- this.$textarea.addEventListener('keyup', () => this.handleKeyUp());
+ this.$textarea.addEventListener('input', () => this.handleInput());
this.$textarea.addEventListener('focus', () => this.handleFocus());
this.$textarea.addEventListener('blur', () => this.handleBlur());
}
- handleKeyUp() {
+ handleInput() {
+ this.updateCount();
this.updateVisibleCountMessage();
this.lastInputTimestamp = Date.now();
}
@@ -970,7 +978,7 @@ class CharacterCount extends ConfigurableComponent {
this.updateScreenReaderCountMessage();
}
updateVisibleCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const isError = remainingNumber < 0;
this.$visibleCountMessage.classList.toggle('govuk-character-count__message--disabled', !this.isOverThreshold());
if (!this.$errorMessage) {
@@ -988,16 +996,18 @@ class CharacterCount extends ConfigurableComponent {
}
this.$screenReaderCountMessage.textContent = this.getCountMessage();
}
- count(text) {
+ updateCount() {
+ const text = this.$textarea.value;
if (this.config.maxwords) {
var _text$match;
const tokens = (_text$match = text.match(/\S+/g)) != null ? _text$match : [];
- return tokens.length;
+ this.count = tokens.length;
+ return;
}
- return text.length;
+ this.count = text.length;
}
getCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const countType = this.config.maxwords ? 'words' : 'characters';
return this.formatCountMessage(remainingNumber, countType);
}
@@ -1014,7 +1024,7 @@ class CharacterCount extends ConfigurableComponent {
if (!this.config.threshold) {
return true;
}
- const currentLength = this.count(this.$textarea.value);
+ const currentLength = this.count;
const maxLength = this.maxLength;
const thresholdValue = maxLength * this.config.threshold / 100;
return thresholdValue <= currentLength;
diff --git a/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.js b/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.js
index 1b6f35f96..437a1ae5e 100644
--- a/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.js
@@ -419,6 +419,7 @@
var _ref, _this$config$maxwords;
super($root, config);
this.$textarea = void 0;
+ this.count = 0;
this.$visibleCountMessage = void 0;
this.$screenReaderCountMessage = void 0;
this.lastInputTimestamp = null;
@@ -474,15 +475,22 @@
$textareaDescription.classList.add('govuk-visually-hidden');
this.$textarea.removeAttribute('maxlength');
this.bindChangeEvents();
- window.addEventListener('pageshow', () => this.updateCountMessage());
+ window.addEventListener('pageshow', () => {
+ if (this.$textarea.value !== this.$textarea.innerHTML) {
+ this.updateCount();
+ this.updateCountMessage();
+ }
+ });
+ this.updateCount();
this.updateCountMessage();
}
bindChangeEvents() {
- this.$textarea.addEventListener('keyup', () => this.handleKeyUp());
+ this.$textarea.addEventListener('input', () => this.handleInput());
this.$textarea.addEventListener('focus', () => this.handleFocus());
this.$textarea.addEventListener('blur', () => this.handleBlur());
}
- handleKeyUp() {
+ handleInput() {
+ this.updateCount();
this.updateVisibleCountMessage();
this.lastInputTimestamp = Date.now();
}
@@ -509,7 +517,7 @@
this.updateScreenReaderCountMessage();
}
updateVisibleCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const isError = remainingNumber < 0;
this.$visibleCountMessage.classList.toggle('govuk-character-count__message--disabled', !this.isOverThreshold());
if (!this.$errorMessage) {
@@ -527,16 +535,18 @@
}
this.$screenReaderCountMessage.textContent = this.getCountMessage();
}
- count(text) {
+ updateCount() {
+ const text = this.$textarea.value;
if (this.config.maxwords) {
var _text$match;
const tokens = (_text$match = text.match(/\S+/g)) != null ? _text$match : [];
- return tokens.length;
+ this.count = tokens.length;
+ return;
}
- return text.length;
+ this.count = text.length;
}
getCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const countType = this.config.maxwords ? 'words' : 'characters';
return this.formatCountMessage(remainingNumber, countType);
}
@@ -553,7 +563,7 @@
if (!this.config.threshold) {
return true;
}
- const currentLength = this.count(this.$textarea.value);
+ const currentLength = this.count;
const maxLength = this.maxLength;
const thresholdValue = maxLength * this.config.threshold / 100;
return thresholdValue <= currentLength;
diff --git a/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.mjs b/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.mjs
index df4f4ca7b..945d744fc 100644
--- a/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/character-count/character-count.bundle.mjs
@@ -413,6 +413,7 @@ class CharacterCount extends ConfigurableComponent {
var _ref, _this$config$maxwords;
super($root, config);
this.$textarea = void 0;
+ this.count = 0;
this.$visibleCountMessage = void 0;
this.$screenReaderCountMessage = void 0;
this.lastInputTimestamp = null;
@@ -468,15 +469,22 @@ class CharacterCount extends ConfigurableComponent {
$textareaDescription.classList.add('govuk-visually-hidden');
this.$textarea.removeAttribute('maxlength');
this.bindChangeEvents();
- window.addEventListener('pageshow', () => this.updateCountMessage());
+ window.addEventListener('pageshow', () => {
+ if (this.$textarea.value !== this.$textarea.innerHTML) {
+ this.updateCount();
+ this.updateCountMessage();
+ }
+ });
+ this.updateCount();
this.updateCountMessage();
}
bindChangeEvents() {
- this.$textarea.addEventListener('keyup', () => this.handleKeyUp());
+ this.$textarea.addEventListener('input', () => this.handleInput());
this.$textarea.addEventListener('focus', () => this.handleFocus());
this.$textarea.addEventListener('blur', () => this.handleBlur());
}
- handleKeyUp() {
+ handleInput() {
+ this.updateCount();
this.updateVisibleCountMessage();
this.lastInputTimestamp = Date.now();
}
@@ -503,7 +511,7 @@ class CharacterCount extends ConfigurableComponent {
this.updateScreenReaderCountMessage();
}
updateVisibleCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const isError = remainingNumber < 0;
this.$visibleCountMessage.classList.toggle('govuk-character-count__message--disabled', !this.isOverThreshold());
if (!this.$errorMessage) {
@@ -521,16 +529,18 @@ class CharacterCount extends ConfigurableComponent {
}
this.$screenReaderCountMessage.textContent = this.getCountMessage();
}
- count(text) {
+ updateCount() {
+ const text = this.$textarea.value;
if (this.config.maxwords) {
var _text$match;
const tokens = (_text$match = text.match(/\S+/g)) != null ? _text$match : [];
- return tokens.length;
+ this.count = tokens.length;
+ return;
}
- return text.length;
+ this.count = text.length;
}
getCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const countType = this.config.maxwords ? 'words' : 'characters';
return this.formatCountMessage(remainingNumber, countType);
}
@@ -547,7 +557,7 @@ class CharacterCount extends ConfigurableComponent {
if (!this.config.threshold) {
return true;
}
- const currentLength = this.count(this.$textarea.value);
+ const currentLength = this.count;
const maxLength = this.maxLength;
const thresholdValue = maxLength * this.config.threshold / 100;
return thresholdValue <= currentLength;
diff --git a/packages/govuk-frontend/dist/govuk/components/character-count/character-count.mjs b/packages/govuk-frontend/dist/govuk/components/character-count/character-count.mjs
index 6466a9647..31cac863d 100644
--- a/packages/govuk-frontend/dist/govuk/components/character-count/character-count.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/character-count/character-count.mjs
@@ -37,6 +37,7 @@ class CharacterCount extends ConfigurableComponent {
var _ref, _this$config$maxwords;
super($root, config);
this.$textarea = void 0;
+ this.count = 0;
this.$visibleCountMessage = void 0;
this.$screenReaderCountMessage = void 0;
this.lastInputTimestamp = null;
@@ -92,15 +93,22 @@ class CharacterCount extends ConfigurableComponent {
$textareaDescription.classList.add('govuk-visually-hidden');
this.$textarea.removeAttribute('maxlength');
this.bindChangeEvents();
- window.addEventListener('pageshow', () => this.updateCountMessage());
+ window.addEventListener('pageshow', () => {
+ if (this.$textarea.value !== this.$textarea.innerHTML) {
+ this.updateCount();
+ this.updateCountMessage();
+ }
+ });
+ this.updateCount();
this.updateCountMessage();
}
bindChangeEvents() {
- this.$textarea.addEventListener('keyup', () => this.handleKeyUp());
+ this.$textarea.addEventListener('input', () => this.handleInput());
this.$textarea.addEventListener('focus', () => this.handleFocus());
this.$textarea.addEventListener('blur', () => this.handleBlur());
}
- handleKeyUp() {
+ handleInput() {
+ this.updateCount();
this.updateVisibleCountMessage();
this.lastInputTimestamp = Date.now();
}
@@ -127,7 +135,7 @@ class CharacterCount extends ConfigurableComponent {
this.updateScreenReaderCountMessage();
}
updateVisibleCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const isError = remainingNumber < 0;
this.$visibleCountMessage.classList.toggle('govuk-character-count__message--disabled', !this.isOverThreshold());
if (!this.$errorMessage) {
@@ -145,16 +153,18 @@ class CharacterCount extends ConfigurableComponent {
}
this.$screenReaderCountMessage.textContent = this.getCountMessage();
}
- count(text) {
+ updateCount() {
+ const text = this.$textarea.value;
if (this.config.maxwords) {
var _text$match;
const tokens = (_text$match = text.match(/\S+/g)) != null ? _text$match : [];
- return tokens.length;
+ this.count = tokens.length;
+ return;
}
- return text.length;
+ this.count = text.length;
}
getCountMessage() {
- const remainingNumber = this.maxLength - this.count(this.$textarea.value);
+ const remainingNumber = this.maxLength - this.count;
const countType = this.config.maxwords ? 'words' : 'characters';
return this.formatCountMessage(remainingNumber, countType);
}
@@ -171,7 +181,7 @@ class CharacterCount extends ConfigurableComponent {
if (!this.config.threshold) {
return true;
}
- const currentLength = this.count(this.$textarea.value);
+ const currentLength = this.count;
const maxLength = this.maxLength;
const thresholdValue = maxLength * this.config.threshold / 100;
return thresholdValue <= currentLength;
Action run for d0b52ec |
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for d0b52ec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separating storing the state from updating the state looks a pretty solid approach to solve the issue 🙌🏻
Regarding the remaining double count, could we maybe use the persisted value from the pageshow event. It seems to indicate when a page is being restored from the cache rather than loaded so looks like it could help distinguish between regular page loads for which pageshow is not relevant and cache restorations for which we want to run the initialisation a second time 🤔
|
@romaricpascal 🤔 I think we did try that out last time we made large changes to the character count (in 2022), but it also didn't work as expected a lot of the time. Still worth a go to see if anything has changed since then, though. |
|
@romaricpascal After (eventually) managing to reproduce the original issue, it doesn't seem like It seems more like it refers to whether the JavaScript state was loaded from cache, not whether the entire page's state was? |
|
@querkmachine Shame that Three other routes that come to mind:
That being said:
So we could happily merge your proposition and log a separate issue to look at the double count from |
I'm not sure this would work how we want it to either. It reduces the initially ran instances of |
6c021bd to
d0b52ec
Compare
An speculative way to resolve #5071 by detaching the method that updates the character/word count value from the methods that update the HTML counters.
This reduces the frequency that the update method is ran, in exchange for needing to be explicit about when the value should be updated. These are:
pageshowevent is triggered, to cover the problem of inconsistent initialisation when navigating back/forward in page history (due to the page's state being stored in the bfcache).inputevent is triggered, caused by typing into the input, pasting content using keyboard or mouse (a situation the current one doesn't support!), etc.This reduces the amount of times that count is carried out on page load from 6 instances—or 10 if a threshold is being used—as documented in #5071, to 2 calls, regardless of whether a threshold is used or not.
By making the
pageshowinstance conditional (by checking whether the textarea'svalueproperty in JS matches itsinnerHTML) we can reduce that to 1 instance on initial page load.From my cursory testing, this seems to work pretty much universally and pass all our tests, but the component is pretty complex, so more thorough manual testing is probably warranted.
Changes
countvariable to the component class which stores the current count value in a method-agnostic manner (code points by default, words ifmaxwordsis set, etc.)countmethod into anupdateCountmethod that is solely responsible for updating thecountvariable.updateCountmethod in the three situations mentioned above.pageshowevent that first checks that the textarea'svalueproperty is different from itsinnerHTML.countmethod to instead use the value of thecountvariable.keyupevent binding toinput, to ensure that pasting content also updates the character count.handleKeyUptohandleInput.Thoughts
We've tried using the
persistedvalue available in thepageshowevent to only re-run initialisation if the page had been restored from the bfcache, in which case the normal initialisation wouldn't happen, however this value seems to only refer to the JavaScript state being persisted, This can lead to situations where the DOM's state has been restored (the character count input has a value entered) but the JavaScript state has not (it thinks the input is empty).