GEOLYTIX / xyz

An open source javascript framework for spatial data and application interfaces.
MIT License
87 stars 25 forks source link

Drawing Instructions Modal #1251

Closed simon-leech closed 1 month ago

simon-leech commented 1 month ago

This PR addresses a common feature request I have been asked for and links to issue https://github.com/GEOLYTIX/xyz/issues/1212. I chose to address 1212 in this way as the user needs to know what options are available when they begin drawing, changing the context menu would only appear at the end of drawing interaction.

Often users aren't clear exactly how to use the drawing interactions. This PR adds a modal element when drawing interactions are started that simply gives guidance on the drawing interaction options available. Hopefully this will just aid user experience a bit as it makes it clearer.

All the text in the modal is using dictionary terms so can be translated. The modal is closable and will close when the drawing interaction finishes or the button state is inactive.

image


simon-leech commented 1 month ago

I start poly, close modal. Click on point and then on poly again. This should be resolved now.

The modal method should be flagged to only show the modal when the flag is set. Moving towards a future where it is possible to store config it can then be possible to remove the flag and not see the modal anymore if no longer desired. I have added the infoModal:true flag that allows the developer and subsequently in the future the user to control the display of this information modal.

The modal should be removed from within the modal method using the data_id. Before a new data_id: 'modal_drawing' is created the existing one will be removed. Added a deleteModal() method to handle this.

Does the modal method require an empty method for the close to work? This is hard to configure in json and should be looked at. There was a bug in the modal.mjs Previously the code was this, which meant passing just true to the close would result in an error.

  function closeModal(e) {
    e.target.closest('.modal').remove();
    modal?.close?.(e);
}

I have updated the function to check on the close value being a function and only run it if so - removing the error.

  function closeModal(e) {
    e.target.closest('.modal').remove();
 // Run the modal close function only if it exists
    if (modal?.close && typeof modal?.close === 'function') {
      modal.close(e);
    }
}

Is it possible to provide the header as html template without the .node calling the web api? Resolved this one.

I have also added conditional text so that the polygon type has a more verbose set of content than the rest

dbauszus-glx commented 1 month ago

I did a few changes for the drawing interactions module but want to revise this more.

I renamed the method to help modal.

Whether the modal should be shown or not should be flagged in the locale. Config. This will allow us to create a plugin which allows the user to turn the modal off for all layers in the locale. And in turn allows the user to store this config in the user locale.

Whether the modal should be shown or not should be controlled within the modal method itself and not whether the modal method is called. Otherwise each call will require a condition check instead of a single condition check.

The deleteModal should use the data-id queryselector. No condition is required this can be always called as nothing will be deleted if there is nothing to delete.

function deleteModal() {
  const modal = document.querySelector('[data-id=modal_drawing]')

  modal && modal.remove()
}

The classlist.toggle returns true on addition and can be used inside the if statement which returns after the interaction finish. No else, add, remove required.

The finish event calls the callback and removes the active class as well as removes the interaction.

Toggling the same interaction on, off, and on again will finish the current interaction and remove the class before starting the interaction. Hence it is required to add the 'active' class and modal after the interaction is triggered.

This is more concise but can further be simplified as the same commands are run for each drawing interaction element.

        const btn = e.target

        if (!btn.classList.toggle('active')) {

          layer.mapview.interaction.finish()
          return;
        }

        !layer.display && layer.show()

        layer.draw.point.callback = feature => {

          layer.draw.callback(feature, layer.draw.point)

          btn.classList.remove('active')

          delete layer.mapview.interaction

          deleteModal();

          // Set highlight interaction if no other interaction is current after 400ms.
          setTimeout(() => {
            !layer.mapview.interaction && layer.mapview.interactions.highlight()
          }, 400)
        }

        layer.mapview.interactions.draw(layer.draw.point)

        helpModal();

        btn.classList.add('active')
dbauszus-glx commented 1 month ago

The drawOnclick method now has all the duplicate code from the individual drawing methods.

function drawOnclick(e, layer, interaction) {

  const btn = e.target

  if (!btn.classList.toggle('active')) {

    layer.mapview.interaction.finish()
    return;
  }

  !layer.display && layer.show()

  interaction.callback = feature => {

    layer.draw.callback(feature, interaction)

    btn.classList.remove('active')

    delete layer.mapview.interaction

    helpModal();

    // Set highlight interaction if no other interaction is current after 400ms.
    setTimeout(() => {
      !layer.mapview.interaction && layer.mapview.interactions.highlight()
    }, 400)
  }

  layer.mapview.interactions.draw(interaction)

  helpModal(interaction.helpModal);

  btn.classList.add('active')
}
dbauszus-glx commented 1 month ago

The helpModal is now a ui.element

let helpModal

export default (params) => {

  // Only one helpModal.node can be shown at any one time.
  helpModal?.node.remove()

  if (!params) return;

  helpModal = {
    target: document.getElementById('Map'),
    height: 'auto',
    width: '200px',
    css_style: 'padding: 0.5em;',
    top: 30,
    left: 60,
    contained: true,
    close: true,
    ...params
  }

  // Create the modal
  mapp.ui.elements.modal(helpModal);
}
dbauszus-glx commented 1 month ago

The drawOnclick method needs to be exported for ease of use in plugins.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

dbauszus-glx commented 1 month ago

The drawing interaction condition requires a wait check to not process inputs while waiting for the geometry function to resolve.

The wait condition will be removed in the API response callback.