Skip to content
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

feat: add text/html support for geoserver #1955

Merged
merged 6 commits into from
Feb 15, 2024

Conversation

Grammostola
Copy link
Contributor

@Grammostola Grammostola commented Jan 29, 2024

Aims to fix #259 for QGIS server and Geoserver (see the second comment: support is only for Geoserver now)

It employs the possible type property of sources in a similar way as print-resize.js (there is a serverType prop of ol/source/wms but it has to do with hiDPI). I like this manual and unambiguous way of specifying a server vendor, it can probably be of use in further places.

The nature of text/html as opposed to a featureCollection means that Origo needs to know where a feature ends and where one starts (for the carousel or multiple features in infowindow) and different vendors have different default templates (and perhaps customizability - ArcGIS Server and Geoserver have customizable templates, I haven't found anything definitive on QGIS server) and as such need to be handled differently. If customized things should not break but that depends on how they are customized.

This PR introduces a htmlseparator layer prop that takes precedence if provided and means what html tag to split features via. If not provided it uses defaults for QGIS server and Geoserver.

The reason ArcGIS Server is left out in the cold with this draft is because its default template, the blue and white horizontal table, gets very wide and breaks the carousel (which doesn't and probably shouldn't provide a scrollbar)

(I also rewrote the function to await syntax because why return a promise chain if no one is forcing you. It did not make the linter unhappy)

The PR sets a new attribute on each feature and on the layer and a new info template that returns the contents of this attribute.

  • Is it of use to support text/html for QGIS server? Or would it be enough with Geoserver?
  • Is there some aspect of the implementation that seems (particularly) objectionable?

text_html_qgiss_geos

@Grammostola
Copy link
Contributor Author

@Grammostola Grammostola marked this pull request as ready for review January 30, 2024 16:02
@Grammostola Grammostola changed the title feat: add prospective text/html support for qgis- and geoserver feat: add text/html support for geoserver Jan 31, 2024
@Grammostola
Copy link
Contributor Author

Thanks to feedback from @johnnyblasta I've a local edit that handles Geoserver's default ftl template.
features_default_geoserver_template

A simple htmlSeparator doesn't work for it as the first <tr> needs to be ignored but the remaining ones treated as features. (Or the first <tr> needs to be combined with each of the others.)

This illustrates my sudden doubts (about adding this functionality to the project):

  • every kind of html needs special handling
  • I can handle the default template even better than this scrot but anyone employing text/html for Geoserver is likely customizing their own featureInfo templates.
  • If htmlSeparator works for our customized templates but not the default template, there's no telling what templates it would work for

The proper solution I see is that the user gets to create their own "htmlHandler" js module but that seems unprecedented.

Is this something that should remain in the forks that need it?

@johnnyblasta
Copy link
Collaborator

Yes, there seems to be a problem to find a solution for all possible cases, but it may suffice to find solution for some standard or simple cases.
Since it would be nice to have the option to use html-template for some advanced case of formatting the attributes.

@johnnyblasta
Copy link
Collaborator

Is it always necessary to have a htmlSepartor?
Maybe a case can be made that all features that is clicked on can be displayed in a table.
htmlSeparator: false

@Grammostola
Copy link
Contributor Author

Cheers for the feedback. Showing the featureinfo result as it "wants" to be shown precludes cycling though individual features in an iw or overlay and seeing the selection switch to match, but it reasonably could be an option I suppose. I'll see what I can do.

@Grammostola
Copy link
Contributor Author

What I've done now follows our discussion yesterday: since it seems practically impossible to find a function that can handle many different htmls (because the htmls can be infinitely varied) we must provide a mechanism for supplying Origo with a handler function.

featureinfo.js now has a addTextHtmlHandler(function) function that can be called via origo.api().getFeatureinfo().addTextHtmlHandler(func) or via a plugin. (It remains until the page is reloaded)

If no such function is provided there is a pre-provided function that will, if no htmlSeparator prop is detected for a layer show the entire html getfeatureinfo response in one card/iw and a dot geom for the click. If a htmlSeparator prop is detected (in form of a html tag to split features by) it will be employed and geometry for the feature/s fetched and shown and carousel/several cards as usual.

A controversial bit might be what isn't user customizable, namely how the html is (re)constructed:

      // case no htmlSeparator prop: show same dot geometry for all hits
      // and put the documentElement of the response within <html>
      if (!layer.get('htmlSeparator')) {
        feature = featureCollection[0];
        htmlfeat = `<html> ${element.documentElement.outerHTML} </html>`;
      } else {
        feature = featureCollection[index];
        htmlfeat = `<html> ${htmlDOM.head.outerHTML} <body> ${element.outerHTML} </body> </html>`;
      }

One can only stray so much from the pre-configured function, so the query here is if the entire if (layer.get('infoFormat') === 'text/html') should be user configurable or if this seems like a decent halfway point between configurability and need to configure.

Copy link
Collaborator

@johnnyblasta johnnyblasta left a comment

Choose a reason for hiding this comment

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

LGTM

Tested with both table and bulleted list and works fine.
The table version doesn't presents well in the overlay, since it is a bit horizontal challenged, but what to do.

@Grammostola
Copy link
Contributor Author

You could attempt to write a fitting handler function :) Thanks for the review, I will merge and create suitable documentation.

@Grammostola Grammostola merged commit 1009814 into master Feb 15, 2024
4 checks passed
@Grammostola Grammostola deleted the text_html_getfeatureinfo_feat branch March 20, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WMS GetFeatureInfo text/html option
2 participants