wegue-oss / wegue

Template and components for webmapping applications with OpenLayers and Vue.js
BSD 2-Clause "Simplified" License
97 stars 41 forks source link

Streamline Wegue Module config conventions #181

Open justb4 opened 3 years ago

justb4 commented 3 years ago

Currently Wegue Modules provided in a Wegue config .json file have a flat list of attributes. But a fixed set of attributes is common, as denoted in the Wegue documentation, like:

    "target": "menu",
    "win": true,
    "draggable": false

Other module-specific attributes seem to be added and handled in an ad-hoc fashion, with custom code. Also in the "Vue-sense" the module-specific attributes should ideally end up in a Component's "props", not in "data".

Within the Geolicious Wegue app for NPLH we developed some more modules and developed a recurring convention. See this branch: https://github.com/Geolicious/wegue/tree/nplh-base and in particular the WguAppTemplate and the config. (Though we also split into "Win" and "Panel" module types, but that is not relevant here). We also found over time many more common attributes.

The convention is simple (though not done for all):

Example:

    "wgu-layerlist": {
      "target": "toolbar",
      "darkLayout": true,
      "panel": true,
      "active": true,
      "mobileActive": false,
      "right": false,
      "width": 360,
      "height": "100%",
      "toggleGroup": "side-panel-active",
      "options": {
        "hideCategories": true,
        "hideTags": false
      }
    },

And how these are passed/used in to LayerListWin: especially:

<template>

  <v-card
    v-draggable-win="draggable" class="wgu-layerlist"
    v-if=show v-bind:style="{ left: left, top: top}"
  >
    <v-toolbar :color="color" :dark="dark">
      <v-toolbar-side-icon><v-icon>{{icon}}</v-icon></v-toolbar-side-icon>
      <v-toolbar-title class="wgu-win-title">{{ $t(title) }}</v-toolbar-title>
      <v-spacer></v-spacer>
      <v-toolbar-side-icon @click="show=false"><v-icon>close</v-icon></v-toolbar-side-icon>
    </v-toolbar>

    <wgu-layerlist v-bind="options" />
.
.
.
   props: {
      dark: {type: Boolean, required: false, default: false},
.
.      initPos: {type: Object, required: false},
      options: { type: Object, required: false, default: {} }

Finally the options like hideCategories end up in LayerList.vue) props without a need to access the $appConfig plus we have sensible default and type-handling:

    props: {
      dark: {type: Boolean, required: false, default: false},
.
.
      hideCategories: { type: Boolean, required: false, default: false },
      hideTags: { type: Boolean, required: false, default: false }
    },

Though it sounds a bit involved, best is to provide a first example. The current core Wegue HelpWinmodule is a good candidate.

Config could become:

    "wgu-helpwin": {
      "target": "toolbar",
      "darkLayout": true, 
      "options": {
            "windowTitle": "About",
            "textTitle": "About Wegue",
            "htmlContent": "<b>WebGIS with OpenLayers and Vue.js</b> Template and re-usable components for webmapping applications with OpenLayers and Vue.js",
            "infoLinkText": "More Info",
            "infoLinkUrl": "http://wegue.org/"
        }
    },

And then in the code we can pass the entire options object to a Panel/Card type Vue component or with e.g. MeasureWin to an OLMeasureController. This is a matter of trying out what works best.

chrismayer commented 3 years ago

That is quite funny that you are coming up with that right now. I had an internal chat with @JakobMiksch about that and we came to the same thoughts that it might be good of having the module configs injected into the corresponding Vue components. This is more "Vue-like" and our motivation was to get rid of the direct binding of the $appConfig object within the module components. Would be useful for a possible separation of the Vue components, but that is another story.

fschmenger commented 3 years ago

I found this issue late. This has been (partially) adressed now. I made some notes in the recently included feature branch #219, so I'm just quoting those:

This change (#219) only affects the properties declared in the 'modules' array of app-conf.json. Some modules, e.g. Maps use global fields from app-conf.json to populate their configuration data. AppTemplate and AppHeader also access the global $appConfig object. We could take the idea of this change-set a step further and consider to access the $appConfig only at top level program initialization. From there we could then rely on properties to pass the configuration through the component tree. IMO this would be nice, however requires a bit more work.

Another little refactoring I propose would be to move the 'darkLayout' property from the individual modules and make it a global setting. Since the hamburger menu now shares the same background color as the toolbar and the window headers, there should be no drawbacks. This probably could be reviewed in conjunction with #202.

fschmenger commented 3 years ago

@chrismayer @justb4 The described generic forwarding mechanism from app-conf properties to module properties is now available as part of #219 (Module specific properties are just flattened inside the module declaration and not wrapped by an explicit optionsproperty). Are we good to close this?

If yes, I recommend to open up a follow up issue, with a subject like "Restrict global appConfig access", which covers some of the remaining topics described in my previous comment. This could also touch long term prospects like supporting multiple appConfig's as part of a MPA (as questionened in #232, if I got it right).