-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add character count on edit document page #166
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
@import "../../../node_modules/govuk-frontend/settings/all"; | ||
@import "../../../node_modules/govuk-frontend/helpers/all"; | ||
@import "../../../node_modules/govuk-frontend/tools/all"; | ||
|
||
.govuk-textarea[data-module="character-count"], | ||
.govuk-input[data-module="character-count"] { | ||
display: block; | ||
margin-bottom: govuk-spacing(1); | ||
} | ||
|
||
.govuk-textarea--error[data-module="character-count"], | ||
.govuk-input--error[data-module="character-count"] { | ||
padding: govuk-spacing(1) - 2; // stop a "jump" when width of border changes | ||
} | ||
|
||
.govuk-character-count__message { | ||
margin-top: govuk-spacing(1); | ||
margin-bottom: 0; | ||
} | ||
|
||
.govuk-character-count__message--disabled { | ||
visibility: hidden; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
function CharacterCount ($module) { | ||
this.$module = $module | ||
} | ||
|
||
CharacterCount.prototype.defaults = { | ||
characterCountAttribute: 'data-maxlength', | ||
wordCountAttribute: 'data-maxwords' | ||
} | ||
|
||
// Initialize component | ||
CharacterCount.prototype.init = function (options) { | ||
// Check for module | ||
var $module = this.$module | ||
if (!$module) { | ||
return | ||
} | ||
|
||
// Read options set using dataset ('data-' values) and merge them with options | ||
var dataset = this.getDataset($module) | ||
this.options = this.merge(options || {}, dataset) | ||
|
||
// Determine the limit attribute (characters or words) | ||
var countAttribute = (this.options.maxwords) ? this.defaults.wordCountAttribute : this.defaults.characterCountAttribute | ||
|
||
// Set the element limit | ||
if ($module.getAttribute) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just quit early here instead of having another if later on? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can quit early here. Maybe with a console message to mention an attribute is missing. Will update. |
||
this.maxLength = $module.getAttribute(countAttribute) | ||
} | ||
|
||
// Generate and reference message | ||
var boundCreateCountMessage = this.createCountMessage.bind(this) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come we need a variable here? |
||
this.countMessage = boundCreateCountMessage() | ||
|
||
// If there's a maximum length defined and the count message was successfully applied | ||
if (this.maxLength && this.countMessage) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, can we return early here instead of having the block? |
||
// Replace maxlength attribute with data-maxlength to avoid hard limit | ||
$module.setAttribute('maxlength', '') | ||
$module.setAttribute('data-maxlength', $module.maxLength) | ||
|
||
// Bind event changes to the textarea | ||
var boundChangeEvents = this.bindChangeEvents.bind(this) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come we need a variable here? |
||
boundChangeEvents() | ||
|
||
// Update count message | ||
var boundUpdateCountMessage = this.updateCountMessage.bind(this) | ||
boundUpdateCountMessage() | ||
} | ||
} | ||
|
||
// Fills in default values | ||
CharacterCount.prototype.merge = function (obj) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be easier to just manually merge attributes? I think there are only two of them ;-) |
||
for (var i = 1; i < arguments.length; i++) { | ||
var def = arguments[i] | ||
for (var n in def) { | ||
if (obj[n] === undefined) obj[n] = def[n] | ||
} | ||
} | ||
return obj | ||
} | ||
|
||
CharacterCount.prototype.getDataset = function (element) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, would it be easier to just fish these out manually if there aren’t many? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or perhaps extract this to somewhere more general as it looks like it could be generally useful :-) |
||
var dataset = {} | ||
var attributes = element.attributes | ||
if (attributes) { | ||
for (var i = 0; i < attributes.length; i++) { | ||
var attribute = attributes[i] | ||
var match = attribute.name.match(/^data-(.+)/) | ||
if (match) { | ||
dataset[match[1]] = attribute.value | ||
} | ||
} | ||
} | ||
return dataset | ||
} | ||
|
||
// Counts characters or words in text | ||
CharacterCount.prototype.count = function (text, options) { | ||
var length | ||
if (options.maxwords) { | ||
var tokens = text.match(/\S+/g) || [] // Matches consecutive non-whitespace chars | ||
length = tokens.length | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just return here rather than have the variable - would be a bit easier to follow then :-) |
||
} else { | ||
length = text.length | ||
} | ||
return length | ||
} | ||
|
||
// Generate count message and bind it to the input | ||
// returns reference to the generated element | ||
CharacterCount.prototype.createCountMessage = function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, wouldn’t it be easier to just the character count as actual HTML like contextual guidance? |
||
var countElement = this.$module | ||
var elementId = countElement.id | ||
// Check for existing info count message | ||
var countMessage = document.getElementById(elementId + '-info') | ||
// If there is no existing info count message we add one right after the field | ||
if (elementId && !countMessage) { | ||
countElement.insertAdjacentHTML('afterend', '<span id="' + elementId + '-info" class="govuk-hint govuk-character-count__message" aria-live="polite"></span>') | ||
this.describedBy = countElement.getAttribute('aria-describedby') | ||
this.describedByInfo = this.describedBy + ' ' + elementId + '-info' | ||
countElement.setAttribute('aria-describedby', this.describedByInfo) | ||
countMessage = document.getElementById(elementId + '-info') | ||
} | ||
return countMessage | ||
} | ||
|
||
// Bind input propertychange to the elements and update based on the change | ||
CharacterCount.prototype.bindChangeEvents = function () { | ||
var $module = this.$module | ||
$module.addEventListener('keyup', this.updateCountMessage.bind(this)) | ||
|
||
// Bind focus/blur events to start/stop polling | ||
$module.addEventListener('focus', this.handleFocus.bind(this)) | ||
$module.addEventListener('blur', this.handleBlur.bind(this)) | ||
} | ||
|
||
// Speech recognition software such as Dragon NaturallySpeaking will modify the | ||
// fields by directly changing its `value`. These changes don't trigger events | ||
// in JavaScript, so we need to poll to handle when and if they occur. | ||
CharacterCount.prototype.checkIfValueChanged = function () { | ||
if (!this.oldValue) this.oldValue = '' | ||
if (this.$module.value !== this.oldValue) { | ||
this.oldValue = this.$module.value | ||
var boundUpdateCountMessage = this.updateCountMessage.bind(this) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a variable here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to store and execute the bounded function so no matter how and when it is called (on which component's instance), is called with a particular this (in our case our component's instance). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm...so is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nope |
||
boundUpdateCountMessage() | ||
} | ||
} | ||
|
||
// Update message box | ||
CharacterCount.prototype.updateCountMessage = function () { | ||
var countElement = this.$module | ||
var options = this.options | ||
var countMessage = this.countMessage | ||
|
||
// Determine the remaining number of characters/words | ||
var currentLength = this.count(countElement.value, options) | ||
var maxLength = this.maxLength | ||
var remainingNumber = maxLength - currentLength | ||
|
||
// Set threshold if presented in options | ||
var threshold = 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could make this a ternary - it seems to be the pattern used in the code above. |
||
if (options.threshold) { | ||
threshold = options.threshold | ||
} | ||
var thresholdValue = maxLength * threshold / 100 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Er, where’s the magic 100 coming from? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if (thresholdValue > currentLength) { | ||
countMessage.classList.add('govuk-character-count__message--disabled') | ||
} else { | ||
countMessage.classList.remove('govuk-character-count__message--disabled') | ||
} | ||
|
||
// Update styles | ||
if (remainingNumber < 0) { | ||
countElement.classList.add('govuk-textarea--error') | ||
countMessage.classList.remove('govuk-hint') | ||
countMessage.classList.add('govuk-error-message') | ||
} else { | ||
countElement.classList.remove('govuk-textarea--error') | ||
countMessage.classList.remove('govuk-error-message') | ||
countMessage.classList.add('govuk-hint') | ||
} | ||
|
||
// Update message | ||
var charVerb = 'remaining' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need these variables? |
||
var charNoun = 'character' | ||
var displayNumber = remainingNumber | ||
if (options.maxwords) { | ||
charNoun = 'word' | ||
} | ||
charNoun = charNoun + ((remainingNumber === -1 || remainingNumber === 1) ? '' : 's') | ||
|
||
charVerb = (remainingNumber < 0) ? 'too many' : 'remaining' | ||
displayNumber = Math.abs(remainingNumber) | ||
|
||
countMessage.innerHTML = 'You have ' + displayNumber + ' ' + charNoun + ' ' + charVerb | ||
} | ||
|
||
CharacterCount.prototype.handleFocus = function () { | ||
// Check if value changed on focus | ||
this.valueChecker = setInterval(this.checkIfValueChanged.bind(this), 1000) | ||
} | ||
|
||
CharacterCount.prototype.handleBlur = function () { | ||
// Cancel value checking on blur | ||
clearInterval(this.valueChecker) | ||
} | ||
|
||
export default CharacterCount |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
name: "document[title]", | ||
value: @document.title, | ||
data: { | ||
"module": "character-count", | ||
"maxlength": "65", | ||
"contextual-guidance": "document-title-guidance" | ||
} | ||
} %> | ||
|
@@ -55,6 +57,8 @@ | |
value: @document.summary, | ||
rows: 4, | ||
data: { | ||
"module": "character-count", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this kind of technique doesn’t extend if we wanted to add more modules to this element - would it be better to attach it in a similar way as contextual guidance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is true, this method allows only one module per container, but generally I would avoid initialising multiple scripts on the same component as they may hijack each other's events. One script per component is consistent with how JavaScript is attached to all the components that are coming from |
||
"maxlength": "160", | ||
"contextual-guidance": "document-summary-guidance" | ||
} | ||
} %> | ||
|
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.
It looks like we’re doing this on characters - why are we supporting words?
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.
It was part of the original script which allows both characters and words. I pulled this in 'as is' because this is how it will come from
govuk-frontend
. Can remove the maxwords for now.