Feature/DF-861 QA fixes (#361)

* Geospatial list - render disabled action links

* Fixes issue where the enter/return keypress activates the map search

* Revert change that treated an empty string as an empty array

* Fix geospatial joi tests

* Code coverage

* Add test coverage for readonly/disabled list

* createFeaturesHTML tests

* Re-render list on cancel

* Code coverage

* Code coverage

Author: davidjamesstone

src/client/javascripts/geospatial-map.js

@@ -164,11 +164,17 @@ export function addFeatureToMap(feature, drawPlugin, map) {
  * Returns HTML summary list for the features
  * @param {FeatureCollection} features - the features
  * @param {string} mapId - the ID of the map
+ * @param {boolean} [disabled] - render the list with disabled links
  * @param {boolean} [readonly] - render the list in readonly mode
  */
-export function createFeaturesHTML(features, mapId, readonly = false) {
+export function createFeaturesHTML(
+  features,
+  mapId,
+  disabled = false,
+  readonly = false
+) {
   return `<dl class="govuk-summary-list">
-    ${features.map((feature, index) => createFeatureHTML(feature, index, mapId, readonly)).join('\n')}
+    ${features.map((feature, index) => createFeatureHTML(feature, index, mapId, disabled, readonly)).join('\n')}
   </dl>`
 }
 
@@ -186,9 +192,16 @@ export function focusFeature(feature, mapProvider) {
  * @param {Feature} feature - the geo feature
  * @param {number} index - the feature index
  * @param {string} mapId - the ID of the map
- * @param {boolean} readonly - render the list item in readonly mode
+ * @param {boolean} [disabled] - render the list with disabled links
+ * @param {boolean} [readonly] - render the list item in readonly mode
  */
-function createFeatureHTML(feature, index, mapId, readonly) {
+export function createFeatureHTML(
+  feature,
+  index,
+  mapId,
+  disabled = false,
+  readonly = false
+) {
   const flattened = feature.geometry.coordinates.flat(2)
 
   const points = []
@@ -203,13 +216,13 @@ function createFeatureHTML(feature, index, mapId, readonly) {
 
   // Change action link
   const changeAction = () => `<li class="govuk-summary-list__actions-list-item">
-  <a class="govuk-link govuk-link--no-visited-state" href="#${mapId}" data-action="edit" data-id="${feature.id}"
+  <a class="govuk-link govuk-link--no-visited-state ${disabled ? 'govuk-link--disabled' : ''}" href="#${mapId}" data-action="edit" data-id="${feature.id}"
     data-type="${feature.geometry.type}">Update<span class="govuk-visually-hidden"> location</span></a>
 </li>`
 
   // Delete action link
   const deleteAction = () => `<li class="govuk-summary-list__actions-list-item">
-  <a class="govuk-link govuk-link--no-visited-state" href="#" data-action="delete" data-id="${feature.id}"
+  <a class="govuk-link govuk-link--no-visited-state ${disabled ? 'govuk-link--disabled' : ''}" href="#" data-action="delete" data-id="${feature.id}"
     data-type="${feature.geometry.type}">Delete<span class="govuk-visually-hidden"> location</span></a>
 </li>`
 
@@ -415,8 +428,8 @@ function getFeaturesManager(geojson) {
  * @returns {RenderList}
  */
 function getListRenderer(geojson, mapId, listEl, renderValue) {
-  return function renderList() {
-    const html = createFeaturesHTML(geojson.features, mapId)
+  return function renderList(disabled = false) {
+    const html = createFeaturesHTML(geojson.features, mapId, disabled)
 
     listEl.innerHTML = html
 
@@ -523,7 +536,7 @@ function createContainers(geospatialInput, index) {
 function onMapReadyFactory(context) {
   const { map, activeFeatureManager, uiManager, interactPlugin, drawPlugin } =
     context
-  const { toggleActionButtons } = uiManager
+  const { toggleActionButtons, renderList } = uiManager
   const { resetActiveFeature } = activeFeatureManager
 
   /**
@@ -542,6 +555,7 @@ function onMapReadyFactory(context) {
       onClick: () => {
         resetActiveFeature()
         toggleActionButtons(true)
+        renderList(true)
         interactPlugin.enable()
       },
       mobile: { slot: 'actions' },
@@ -556,6 +570,7 @@ function onMapReadyFactory(context) {
       onClick: () => {
         resetActiveFeature()
         toggleActionButtons(true)
+        renderList(true)
         drawPlugin.newPolygon(generateID(), polygonFeatureProperties)
       },
       mobile: { slot: 'actions' },
@@ -570,6 +585,7 @@ function onMapReadyFactory(context) {
       onClick: () => {
         resetActiveFeature()
         toggleActionButtons(true)
+        renderList(true)
         drawPlugin.newLine(generateID(), lineFeatureProperties)
       },
       mobile: { slot: 'actions' },
@@ -589,6 +605,7 @@ function onMapReadyFactory(context) {
     const { listEl } = uiManager
     listEl.addEventListener('click', onListElClickFactory(context), false)
     listEl.addEventListener('change', onListElChangeFactory(context), false)
+    listEl.addEventListener('keydown', onListElKeydownFactory(), false)
   }
 }
 
@@ -679,7 +696,7 @@ function onDrawEditedFactory(context) {
  */
 function onDrawCancelledFactory(context) {
   const { uiManager, activeFeatureManager } = context
-  const { toggleActionButtons } = uiManager
+  const { toggleActionButtons, renderList } = uiManager
   const { resetActiveFeature } = activeFeatureManager
 
   /**
@@ -688,6 +705,7 @@ function onDrawCancelledFactory(context) {
   return function onDrawCancelled() {
     toggleActionButtons(false)
     resetActiveFeature()
+    renderList()
   }
 }
 
@@ -815,6 +833,7 @@ function onListElClickFactory(context) {
     }
 
     toggleActionButtons(true)
+    renderList(true)
   }
 
   /**
@@ -859,7 +878,7 @@ function onListElClickFactory(context) {
 }
 
 /**
- * Callback factory function that fires a 'change' event is fired on the list container
+ * Callback factory function that fires when a 'change' event is fired on the list container
  * @param {Context} context - the UI context
  */
 function onListElChangeFactory(context) {
@@ -891,6 +910,29 @@ function onListElChangeFactory(context) {
   }
 }
 
+/**
+ * Callback factory function that fires when a 'keydown' event is fired on the list container
+ */
+function onListElKeydownFactory() {
+  /**
+   * List container delegated 'keydown' events handler
+   * Fixes the issue of pressing "Enter" key in the description input triggering the map search
+   * @param {KeyboardEvent} e
+   */
+  return function (e) {
+    const target = e.target
+
+    if (!(target instanceof HTMLInputElement)) {
+      return
+    }
+
+    if (e.code === 'Enter' || e.code === 'NumpadEnter') {
+      e.preventDefault()
+      e.stopPropagation()
+    }
+  }
+}
+
 /**
  * @import { MapsEnvironmentConfig, InteractiveMap } from '~/src/client/javascripts/map.js'
  */
@@ -960,6 +1002,7 @@ function onListElChangeFactory(context) {
 /**
  * Renders the features into the list
  * @callback RenderList
+ * @param {boolean} [disabled] - whether to render the list with disabled links
  * @returns {void}
  */
 

src/client/stylesheets/shared.scss

@@ -33,6 +33,11 @@
   @include govuk-font($size: 19);
 }
 
+// Used in geospatial field
+.govuk-link--disabled {
+  opacity: 0.5;
+}
+
 // Hide urls for hyperlinks when in print mode
 @media print {
   .govuk-link[href]::after {

src/server/plugins/engine/components/GeospatialField.test.ts

@@ -110,7 +110,7 @@ describe('GeospatialField', () => {
 
         expect(result.errors).toEqual([
           expect.objectContaining({
-            text: 'Example geospatial must contain at least 1 items'
+            text: 'Select example geospatial'
           })
         ])
       })
@@ -128,7 +128,7 @@ describe('GeospatialField', () => {
 
         expect(result.errors).toEqual([
           expect.objectContaining({
-            text: 'Example geospatial title must contain at least 1 items'
+            text: 'Select example geospatial title'
           })
         ])
       })

src/server/plugins/engine/components/helpers/geospatial.test.js

@@ -49,14 +49,7 @@ describe('Geospatial validation helpers', () => {
   test('it should validate an empty string', () => {
     const result = geospatialSchema.validate('')
 
-    expect(result.error).toBeUndefined()
-    expect(result.value).toEqual([])
-  })
-
-  test('it should validate an empty string with errors when required', () => {
-    const result = geospatialSchema.min(1).required().validate('')
-
     expect(result.error).toBeDefined()
-    expect(result.value).toEqual([])
+    expect(result.value).toBeUndefined()
   })
 })

src/server/plugins/engine/components/helpers/geospatial.ts

@@ -20,7 +20,7 @@ const Joi = JoiBase.extend({
       if (typeof value === 'string') {
         if (value.trim() === '') {
           return {
-            value: []
+            value: undefined
           }
         }
 

test/client/javascripts/map.test.js

@@ -1,3 +1,7 @@
+import {
+  createFeatureHTML,
+  createFeaturesHTML
+} from '~/src/client/javascripts/geospatial-map.js'
 import {
   formSubmitFactory,
   getCentroidGridRef,
@@ -1199,6 +1203,107 @@ describe('Maps Client JS', () => {
           ...features.slice(1)
         ])
       })
+
+      test('description enter/return key', () => {
+        const { geospatialInput, listContainer } = initialiseGeospatialMaps()
+
+        // Manually change the description
+        const buckinghamPalaceInputEl = getDescription(listContainer, 0)
+
+        buckinghamPalaceInputEl.value = 'New description'
+        buckinghamPalaceInputEl.dispatchEvent(
+          new window.Event('change', { bubbles: true })
+        )
+        buckinghamPalaceInputEl.dispatchEvent(
+          new window.KeyboardEvent('keydown', { bubbles: true, code: 'Enter' })
+        )
+
+        expect(JSON.parse(geospatialInput.value)).toEqual([
+          {
+            ...features[0],
+            properties: {
+              description: 'New description',
+              coordinateGridReference: 'TQ 29031 79662',
+              centroidGridReference: 'TQ 29031 79662'
+            }
+          },
+          ...features.slice(1)
+        ])
+      })
+
+      test('description enter/return numpad key', () => {
+        const { geospatialInput, listContainer } = initialiseGeospatialMaps()
+
+        // Manually change the description
+        const buckinghamPalaceInputEl = getDescription(listContainer, 0)
+
+        buckinghamPalaceInputEl.value = 'New description'
+        buckinghamPalaceInputEl.dispatchEvent(
+          new window.Event('change', { bubbles: true })
+        )
+        buckinghamPalaceInputEl.dispatchEvent(
+          new window.KeyboardEvent('keydown', {
+            bubbles: true,
+            code: 'NumpadEnter'
+          })
+        )
+
+        expect(JSON.parse(geospatialInput.value)).toEqual([
+          {
+            ...features[0],
+            properties: {
+              description: 'New description',
+              coordinateGridReference: 'TQ 29031 79662',
+              centroidGridReference: 'TQ 29031 79662'
+            }
+          },
+          ...features.slice(1)
+        ])
+      })
+
+      test('createFeaturesHTML - normal', () => {
+        const html = createFeaturesHTML(features.slice(1), 'test', false, false)
+
+        expect(html).toContain('data-action="delete"')
+        expect(html).toContain('data-action="edit"')
+        expect(html).not.toContain('govuk-link--disabled')
+        expect(html).not.toContain('data-action="focus"')
+      })
+
+      test('createFeaturesHTML - normal (defaults)', () => {
+        const html = createFeaturesHTML(features.slice(1), 'test')
+
+        expect(html).toContain('data-action="delete"')
+        expect(html).toContain('data-action="edit"')
+        expect(html).not.toContain('govuk-link--disabled')
+        expect(html).not.toContain('data-action="focus"')
+      })
+
+      test('createFeaturesHTML - readonly (for use in designer viewer)', () => {
+        const html = createFeaturesHTML(features.slice(1), 'test', false, true)
+
+        expect(html).not.toContain('data-action="delete"')
+        expect(html).not.toContain('data-action="edit"')
+        expect(html).toContain('data-action="focus"')
+      })
+
+      test('createFeaturesHTML - disabled', () => {
+        const html = createFeaturesHTML(features.slice(1), 'test', true, false)
+
+        expect(html).toContain('govuk-link--disabled')
+        expect(html).toContain('data-action="delete"')
+        expect(html).toContain('data-action="edit"')
+        expect(html).not.toContain('data-action="focus"')
+      })
+
+      test('createFeatureHTML - normal (defaults)', () => {
+        const html = createFeatureHTML(features[1], 0, 'test')
+
+        expect(html).toContain('data-action="delete"')
+        expect(html).toContain('data-action="edit"')
+        expect(html).not.toContain('govuk-link--disabled')
+        expect(html).not.toContain('data-action="focus"')
+      })
     })
   })