[Fix] Don't modify height for with fixed height enabled; Height UI un…

…ification (#2804) Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
keplergl · Dec 6, 2024 · 786aa36 · 786aa36
1 parent 2178d90
commit 786aa36
Show file tree

Hide file tree

Showing 21 changed files with 68 additions and 47 deletions.
diff --git a/src/components/src/side-panel/layer-panel/aggr-scale-selector.tsx b/src/components/src/side-panel/layer-panel/aggr-scale-selector.tsx
@@ -6,15 +6,15 @@ import DimensionScaleSelectorFactory from './dimension-scale-selector';
 
 AggrScaleSelectorFactory.deps = [DimensionScaleSelectorFactory];
 export function AggrScaleSelectorFactory(DimensionScaleSelector) {
-  const AggrScaleSelector = ({channel, layer, onChange, setColorUI}) => {
+  const AggrScaleSelector = ({channel, layer, onChange, setColorUI, label}) => {
     const {key} = channel;
     const scaleOptions = layer.getScaleOptions(key);
 
     return Array.isArray(scaleOptions) && scaleOptions.length > 1 ? (
       <DimensionScaleSelector
         layer={layer}
         channel={channel}
-        label={`${key} Scale`}
+        label={label || `${key} Scale`}
         onChange={onChange}
         setColorUI={setColorUI}
       />

diff --git a/src/components/src/side-panel/layer-panel/layer-configurator.tsx b/src/components/src/side-panel/layer-panel/layer-configurator.tsx
@@ -353,7 +353,7 @@ export default function LayerConfiguratorFactory(
     }) {
       const {config} = layer;
       const {
-        visConfig: {enable3d}
+        visConfig: {enable3d, fixedHeight}
       } = config;
       const elevationByDescription = 'layer.elevationByDescription';
       const colorByDescription = 'layer.colorByDescription';
@@ -409,7 +409,7 @@ export default function LayerConfiguratorFactory(
               <VisConfigSlider
                 {...layer.visConfigSettings.elevationScale}
                 {...visConfiguratorProps}
-                label="layerVisConfigs.heightMultiplier"
+                label={'layerVisConfigs.heightMultiplier'}
               />
               <ConfigGroupCollapsibleContent>
                 <ChannelByValueSelector
@@ -418,27 +418,28 @@ export default function LayerConfiguratorFactory(
                   description={elevationByDescription}
                   disabled={!enable3d}
                 />
+                {layer.visConfigSettings.sizeAggregation.condition(layer.config) ? (
+                  <AggregationTypeSelector
+                    {...layer.visConfigSettings.sizeAggregation}
+                    {...layerChannelConfigProps}
+                    channel={layer.visualChannels.size}
+                  />
+                ) : null}
                 <AggrScaleSelector
                   {...layerConfiguratorProps}
                   channel={layer.visualChannels.size}
+                  label={'Height Scale'}
                 />
                 <VisConfigSlider
                   {...layer.visConfigSettings.sizeRange}
                   {...visConfiguratorProps}
-                  label="layerVisConfigs.heightRange"
+                  label={'layerVisConfigs.heightRange'}
+                  disabled={fixedHeight}
                 />
                 <VisConfigSwitch
-                  {...layer.visConfigSettings.enableElevationZoomFactor}
+                  {...layer.visConfigSettings.fixedHeight}
                   {...visConfiguratorProps}
-                  label="layerVisConfigs.enableHeightZoomFactor"
                 />
-                {layer.visConfigSettings.sizeAggregation.condition(layer.config) ? (
-                  <AggregationTypeSelector
-                    {...layer.visConfigSettings.sizeAggregation}
-                    {...layerChannelConfigProps}
-                    channel={layer.visualChannels.size}
-                  />
-                ) : null}
                 {layer.visConfigSettings.elevationPercentile.condition(layer.config) ? (
                   <VisConfigSlider
                     {...layer.visConfigSettings.elevationPercentile}
@@ -552,20 +553,18 @@ export default function LayerConfiguratorFactory(
               channel={layer.visualChannels.size}
               {...layerChannelConfigProps}
             />
+            <VisConfigSlider
+              {...layer.visConfigSettings.elevationScale}
+              {...visConfiguratorProps}
+              label={'layerVisConfigs.heightMultiplier'}
+            />
             <ConfigGroupCollapsibleContent>
-              <VisConfigSlider
-                {...layer.visConfigSettings.elevationScale}
-                {...visConfiguratorProps}
-              />
               <VisConfigSlider
                 {...layer.visConfigSettings.sizeRange}
                 {...visConfiguratorProps}
                 label="layerVisConfigs.heightRange"
               />
-              <VisConfigSwitch
-                {...layer.visConfigSettings.enableElevationZoomFactor}
-                {...visConfiguratorProps}
-              />
+              <VisConfigSwitch {...layer.visConfigSettings.fixedHeight} {...visConfiguratorProps} />
             </ConfigGroupCollapsibleContent>
           </LayerConfigGroup>
 
@@ -1006,18 +1005,15 @@ export default function LayerConfiguratorFactory(
             <VisConfigSlider
               {...layer.visConfigSettings.elevationScale}
               {...visConfiguratorProps}
-              label="layerVisConfigs.elevationScale"
+              label={'layerVisConfigs.heightMultiplier'}
             />
             <ConfigGroupCollapsibleContent>
               <VisConfigSlider
                 {...layer.visConfigSettings.heightRange}
                 {...visConfiguratorProps}
                 label="layerVisConfigs.heightRange"
               />
-              <VisConfigSwitch
-                {...layer.visConfigSettings.enableElevationZoomFactor}
-                {...visConfiguratorProps}
-              />
+              <VisConfigSwitch {...layer.visConfigSettings.fixedHeight} {...visConfiguratorProps} />
               <VisConfigSwitch {...visConfiguratorProps} {...layer.visConfigSettings.wireframe} />
             </ConfigGroupCollapsibleContent>
           </LayerConfigGroup>

diff --git a/src/constants/src/layers.ts b/src/constants/src/layers.ts
@@ -348,7 +348,7 @@ export const LAYER_VIS_CONFIGS: LayerVisConfigSettings = {
     defaultValue: 5,
     label: 'layerVisConfigs.elevationScale',
     isRanged: false,
-    range: [0, 100],
+    range: [0, 1000],
     step: 0.1,
     group: PROPERTY_GROUPS.height,
     property: 'elevationScale',

diff --git a/src/deckgl-layers/src/layer-utils/cpu-aggregator.ts b/src/deckgl-layers/src/layer-utils/cpu-aggregator.ts
@@ -138,7 +138,7 @@ export function getDimensionValueDomain(this: CPUAggregator, step, props, dimens
 
 export function getDimensionScale(this: CPUAggregator, step, props, dimensionUpdater) {
   const {key} = dimensionUpdater;
-  const {domain, range, scaleType} = step.triggers;
+  const {domain, range, scaleType, fixed} = step.triggers;
   const {onSet} = step;
   if (!this.state.dimensions[key].valueDomain) {
     // the previous step should set valueDomain, if not, something went wrong
@@ -147,10 +147,13 @@ export function getDimensionScale(this: CPUAggregator, step, props, dimensionUpd
 
   const dimensionRange = props[range.prop];
   const dimensionDomain = props[domain.prop] || this.state.dimensions[key].valueDomain;
+  const dimensionFixed = Boolean(fixed && props[fixed.prop]);
 
   const scaleFunctor = getScaleFunctor(scaleType && props[scaleType.prop])();
 
-  const scaleFunc = scaleFunctor.domain(dimensionDomain).range(dimensionRange);
+  const scaleFunc = scaleFunctor
+    .domain(dimensionDomain)
+    .range(dimensionFixed ? dimensionDomain : dimensionRange);
 
   if (typeof onSet === 'object' && typeof props[onSet.props] === 'function') {
     props[onSet.props](scaleFunc.domain());
@@ -356,6 +359,7 @@ export const defaultElevationDimension: DimensionType<number> = {
     {
       key: 'getScaleFunc',
       triggers: {
+        fixed: {prop: 'elevationFixed'},
         domain: {prop: 'elevationDomain'},
         range: {prop: 'elevationRange'},
         scaleType: {prop: 'elevationScaleType'}

diff --git a/src/layers/src/aggregation-layer.ts b/src/layers/src/aggregation-layer.ts
@@ -114,7 +114,8 @@ export default class AggregationLayer extends Layer {
       'coverage',
       'elevationPercentile',
       'elevationScale',
-      'enableElevationZoomFactor'
+      'enableElevationZoomFactor',
+      'fixedHeight'
     ];
   }
 
@@ -375,6 +376,8 @@ export default class AggregationLayer extends Layer {
       elevationScale: visConfig.elevationScale * eleZoomFactor,
       elevationScaleType: this.config.sizeScale,
       elevationRange: visConfig.sizeRange,
+      elevationFixed: visConfig.fixedHeight,
+
       elevationLowerPercentile: visConfig.elevationPercentile[0],
       elevationUpperPercentile: visConfig.elevationPercentile[1],
 

diff --git a/src/layers/src/base-layer.ts b/src/layers/src/base-layer.ts
@@ -644,9 +644,11 @@ class Layer {
    * @returns {number}
    */
   getElevationZoomFactor({zoom, zoomOffset = 0}: {zoom: number; zoomOffset?: number}): number {
-    return this.config.visConfig.enableElevationZoomFactor
-      ? Math.pow(2, Math.max(8 - zoom + zoomOffset, 0))
-      : 1;
+    // enableElevationZoomFactor is used to support existing maps
+    const {fixedHeight, enableElevationZoomFactor} = this.config.visConfig;
+    return fixedHeight || enableElevationZoomFactor === false
+      ? 1
+      : Math.pow(2, Math.max(8 - zoom + zoomOffset, 0));
   }
 
   // eslint-disable-next-line @typescript-eslint/no-unused-vars

diff --git a/src/layers/src/grid-layer/grid-layer.ts b/src/layers/src/grid-layer/grid-layer.ts
@@ -58,6 +58,7 @@ export const gridVisConfigs: {
   elevationPercentile: 'elevationPercentile';
   elevationScale: 'elevationScale';
   enableElevationZoomFactor: 'enableElevationZoomFactor';
+  fixedHeight: 'fixedHeight';
   colorAggregation: 'colorAggregation';
   sizeAggregation: 'sizeAggregation';
   enable3d: 'enable3d';
@@ -71,6 +72,7 @@ export const gridVisConfigs: {
   elevationPercentile: 'elevationPercentile',
   elevationScale: 'elevationScale',
   enableElevationZoomFactor: 'enableElevationZoomFactor',
+  fixedHeight: 'fixedHeight',
   colorAggregation: 'colorAggregation',
   sizeAggregation: 'sizeAggregation',
   enable3d: 'enable3d'

diff --git a/src/layers/src/h3-hexagon-layer/h3-hexagon-layer.ts b/src/layers/src/h3-hexagon-layer/h3-hexagon-layer.ts
@@ -116,6 +116,7 @@ export const HexagonIdVisConfigs: {
   coverageRange: 'coverageRange';
   elevationScale: 'elevationScale';
   enableElevationZoomFactor: 'enableElevationZoomFactor';
+  fixedHeight: 'fixedHeight';
 } = {
   colorRange: 'colorRange',
   filled: {
@@ -143,7 +144,8 @@ export const HexagonIdVisConfigs: {
   sizeRange: 'elevationRange',
   coverageRange: 'coverageRange',
   elevationScale: 'elevationScale',
-  enableElevationZoomFactor: 'enableElevationZoomFactor'
+  enableElevationZoomFactor: 'enableElevationZoomFactor',
+  fixedHeight: 'fixedHeight'
 };
 
 const brushingExtension = new BrushingExtension();

diff --git a/src/layers/src/hexagon-layer/hexagon-layer.ts b/src/layers/src/hexagon-layer/hexagon-layer.ts
@@ -62,6 +62,7 @@ export const hexagonVisConfigs: {
   elevationPercentile: 'elevationPercentile';
   elevationScale: 'elevationScale';
   enableElevationZoomFactor: 'enableElevationZoomFactor';
+  fixedHeight: 'fixedHeight';
   colorAggregation: 'colorAggregation';
   sizeAggregation: 'sizeAggregation';
   enable3d: 'enable3d';
@@ -76,6 +77,7 @@ export const hexagonVisConfigs: {
   elevationPercentile: 'elevationPercentile',
   elevationScale: 'elevationScale',
   enableElevationZoomFactor: 'enableElevationZoomFactor',
+  fixedHeight: 'fixedHeight',
   colorAggregation: 'colorAggregation',
   sizeAggregation: 'sizeAggregation',
   enable3d: 'enable3d'

diff --git a/src/layers/src/s2-geometry-layer/s2-geometry-layer.ts b/src/layers/src/s2-geometry-layer/s2-geometry-layer.ts
@@ -118,6 +118,7 @@ export const S2VisConfigs: {
   enable3d: 'enable3d';
   elevationScale: 'elevationScale';
   enableElevationZoomFactor: 'enableElevationZoomFactor';
+  fixedHeight: 'fixedHeight';
   heightRange: 'elevationRange';
 
   // wireframe
@@ -148,6 +149,7 @@ export const S2VisConfigs: {
   enable3d: 'enable3d',
   elevationScale: 'elevationScale',
   enableElevationZoomFactor: 'enableElevationZoomFactor',
+  fixedHeight: 'fixedHeight',
   heightRange: 'elevationRange',
 
   // wireframe

diff --git a/src/localization/src/translations/en.ts b/src/localization/src/translations/en.ts
@@ -159,12 +159,12 @@ export default {
     weightIntensity: 'Weight Intensity',
     zoomScale: 'Zoom Scale',
     heightRange: 'Height Range',
+    heightMultiplier: 'Height Multiplier',
     fixedHeight: 'Fixed height',
     fixedHeightDescription: 'Use height without modifications',
     allowHover: 'Allow Hover',
     showNeighborOnHover: 'Highlight Neighbors On Hover',
     showHighlightColor: 'Show highlight Color',
-    heightMultiplier: 'Height Multiplier',
     darkModeEnabled: 'Dark base map'
   },
   layerManager: {

diff --git a/src/localization/src/translations/es.ts b/src/localization/src/translations/es.ts
@@ -151,9 +151,9 @@ export default {
     weightIntensity: 'Intensidad de peso',
     zoomScale: 'Escala de zoom',
     heightRange: 'Rango de alturas',
+    heightMultiplier: 'Multiplicador de altura',
     fixedHeight: 'Altura fija',
-    fixedHeightDescription: 'Usar altura sin modificaciones',
-    heightMultiplier: 'Multiplicador de altura'
+    fixedHeightDescription: 'Usar altura sin modificaciones'
   },
   layerManager: {
     addData: 'Añadir datos',

diff --git a/src/localization/src/translations/fi.ts b/src/localization/src/translations/fi.ts
@@ -148,9 +148,9 @@ export default {
     weightIntensity: 'Painotuksen intensiteetti',
     zoomScale: 'Zoomausskaala',
     heightRange: 'Korkeuden rajat',
+    heightMultiplier: 'Korkeuskerroin',
     fixedHeight: 'Kiinteä korkeus',
-    fixedHeightDescription: 'Käytä korkeutta ilman muutoksia',
-    heightMultiplier: 'Korkeuskerroin'
+    fixedHeightDescription: 'Käytä korkeutta ilman muutoksia'
   },
   layerManager: {
     addData: 'Lisää aineisto',

diff --git a/src/localization/src/translations/ja.ts b/src/localization/src/translations/ja.ts
@@ -147,9 +147,9 @@ export default {
     weightIntensity: '重みづけの強さ',
     zoomScale: 'ズームのスケール',
     heightRange: '高さの範囲',
+    heightMultiplier: '高さ乗数',
     fixedHeight: '固定高さ',
-    fixedHeightDescription: '高さを変更せずに使用する',
-    heightMultiplier: '高さ乗数'
+    fixedHeightDescription: '高さを変更せずに使用する'
   },
   layerManager: {
     addData: 'データ追加',

diff --git a/src/localization/src/translations/pt.ts b/src/localization/src/translations/pt.ts
@@ -149,9 +149,9 @@ export default {
     weightIntensity: 'Intensidade da Espessura',
     zoomScale: 'Escala do Zoom',
     heightRange: 'Alcance da Altura',
+    heightMultiplier: 'Multiplicador de altura',
     fixedHeight: 'Altura fixa',
-    fixedHeightDescription: 'Use a altura sem modificações',
-    heightMultiplier: 'Multiplicador de altura'
+    fixedHeightDescription: 'Use a altura sem modificações'
   },
   layerManager: {
     addData: 'Adicionar Dados',

diff --git a/test/browser/layer-tests/s2-geometry-layer-specs.js b/test/browser/layer-tests/s2-geometry-layer-specs.js
@@ -43,6 +43,7 @@ test('#S2Geometry -> constructor', t => {
             'enable3d',
             'elevationScale',
             'enableElevationZoomFactor',
+            'fixedHeight',
             'heightRange',
             'wireframe'
           ],

diff --git a/test/fixtures/state-saved-v0.js b/test/fixtures/state-saved-v0.js
@@ -1297,7 +1297,8 @@ mergedLayer3.config = {
     percentile: [0, 100],
     elevationPercentile: [0, 100],
     elevationScale: 10,
-    enableElevationZoomFactor: true
+    enableElevationZoomFactor: true,
+    fixedHeight: false
   },
   animation: {enabled: false}
 };

diff --git a/test/fixtures/state-saved-v1-3.js b/test/fixtures/state-saved-v1-3.js
@@ -458,6 +458,7 @@ mergedLayer1.config = {
     elevationPercentile: [0, 100],
     elevationScale: 5,
     enableElevationZoomFactor: true,
+    fixedHeight: false,
     colorAggregation: 'count', // if associated colorField value is null, it can only default to 'count'
     sizeAggregation: 'count', // if associated sizeField value is null, it can only default to 'count'
     enable3d: false

diff --git a/test/fixtures/test-csv-data.js b/test/fixtures/test-csv-data.js
@@ -141,6 +141,7 @@ export const config = {
               elevationPercentile: [0, 100],
               elevationScale: 5,
               enableElevationZoomFactor: true,
+              fixedHeight: false,
               colorAggregation: 'count',
               sizeAggregation: 'count',
               enable3d: false

diff --git a/test/fixtures/test-hex-id-data.js b/test/fixtures/test-hex-id-data.js
@@ -76,7 +76,8 @@ export const hexIdDataConfig = {
                 sizeRange: [0, 500],
                 coverageRange: [0, 1],
                 elevationScale: 5,
-                enableElevationZoomFactor: true
+                enableElevationZoomFactor: true,
+                fixedHeight: false
               },
               textLabel: [
                 {
@@ -269,7 +270,8 @@ mergedH3Layer.config = {
     coverageRange: [0, 1],
     enable3d: false,
     elevationScale: 5,
-    enableElevationZoomFactor: true
+    enableElevationZoomFactor: true,
+    fixedHeight: false
   },
   textLabel: [
     {

diff --git a/test/helpers/mock-state.js b/test/helpers/mock-state.js
@@ -914,6 +914,7 @@ export const expectedSavedLayer0 = {
       elevationPercentile: [0, 100],
       elevationScale: 5,
       enableElevationZoomFactor: true,
+      fixedHeight: false,
       colorAggregation: 'count',
       sizeAggregation: 'count',
       enable3d: false
@@ -953,6 +954,7 @@ export const expectedLoadedLayer0 = {
       elevationPercentile: [0, 100],
       elevationScale: 5,
       enableElevationZoomFactor: true,
+      fixedHeight: false,
       colorAggregation: 'count',
       sizeAggregation: 'count',
       enable3d: false