fix: truncate map tooltips - #3299
Conversation
| // Returns true if the value is likely a WKT geometry string (heuristic check). | ||
| const WKT_PREFIX_RE = | ||
| /^(?:SRID=\d+\s*;\s*)?(?:POINT|LINESTRING|POLYGON|MULTIPOINT|MULTILINESTRING|MULTIPOLYGON|GEOMETRYCOLLECTION)(?:\s+(?:Z|M|ZM))?\s*\(/i; |
There was a problem hiding this comment.
I am rollbacking this previous change
and applying a simpler truncate label
| test('datasetUtils.isWkt', t => { | ||
| // non-strings | ||
| t.notOk(isWkt(null), 'null is not a valid WKT'); |
There was a problem hiding this comment.
I am rollbacking this previous change
and applying a simpler truncate label
2070e0f to
952aa78
Compare
There was a problem hiding this comment.
Pull request overview
Updates hover tooltip rendering to prevent extremely long string values from overflowing, but also includes unrelated removals in common-utils (WKT detection) and associated tests.
Changes:
- Truncate long tooltip values in
LayerHoverInforows usinglodash/truncate. - Remove
isWkthelper and WKT-based geometry inference from common-utils. - Remove WKT-related tests (and inadvertently remove the
isHexWkbimport used by remaining tests).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/node/utils/dataset-utils-test.js | Removes common-utils imports and deletes the isWkt test block (leaving isHexWkb references without an import). |
| src/components/src/map/layer-hover-info.tsx | Truncates long string values before rendering in hover tooltip rows. |
| src/common-utils/src/data-type.ts | Removes isWkt export and stops inferring geometry-from-string via WKT in getFieldsFromData. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const H3_ANALYZER_TYPE = 'H3'; | ||
|
|
||
| // Returns true if the value is likely a WKT geometry string (heuristic check). | ||
| const WKT_PREFIX_RE = | ||
| /^(?:SRID=\d+\s*;\s*)?(?:POINT|LINESTRING|POLYGON|MULTIPOINT|MULTILINESTRING|MULTIPOLYGON|GEOMETRYCOLLECTION)(?:\s+(?:Z|M|ZM))?\s*\(/i; | ||
|
|
||
| export function isWkt(value: unknown): boolean { | ||
| if (typeof value !== 'string') { | ||
| return false; | ||
| } | ||
|
|
||
| const s = value.trim(); | ||
| if (s.length < 10) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!s.includes('(') || !s.includes(')')) { | ||
| return false; | ||
| } | ||
|
|
||
| return WKT_PREFIX_RE.test(s); | ||
| } | ||
|
|
||
| export const ACCEPTED_ANALYZER_TYPES = [ | ||
| AnalyzerDATA_TYPES.DATE, | ||
| AnalyzerDATA_TYPES.TIME, |
There was a problem hiding this comment.
This change removes the exported isWkt helper from @kepler.gl/common-utils (it was previously re-exported via src/common-utils/src/index.ts). That’s a breaking public API change and is also unrelated to the PR’s stated goal (“truncate map tooltips”). Consider restoring isWkt (or keeping a deprecated re-export) and moving any API removals into a dedicated major-version PR.
There was a problem hiding this comment.
We are rollbacking this, no need for isWkt which was added in previous PR
|
|
||
| // quick check if string is hex wkb | ||
| if (type === AnalyzerDATA_TYPES.STRING) { | ||
| type = data.some(d => isHexWkb(d[name])) ? AnalyzerDATA_TYPES.GEOMETRY : type; | ||
| } |
There was a problem hiding this comment.
getFieldsFromData no longer performs the WKT quick-check and therefore won’t detect WKT geometry stored as strings (only hex WKB). This is a behavior regression for datasets that contain WKT columns and can prevent geometry layers from being created. If WKT support is still desired, restore the WKT detection here (or replace it with an equivalent) and keep/adjust the associated tests.
There was a problem hiding this comment.
We are rollbacking this, no need for isWkt which was added in previous PR
…ds (keplergl#3294) * fix tooltip coordinate Signed-off-by: bdjulbic <bdjulbic@foursquare.com> * reformat and add test Signed-off-by: bdjulbic <bdjulbic@foursquare.com> --------- Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
…gl#3297) Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
c114861 to
14ea63d
Compare
…oltips # Conflicts: # test/node/utils/dataset-utils-test.js
Rollback of #3298
The previous fix for hiding geometry fields in tooltips wasn't covering all use cases.
Problem: Geometry fields loaded as strings via CSV (displayed as vector tilesets) show excessively long WKT values in map tooltips.
Solution: Apply truncation to tooltip values exceeding 60 characters, shortening them to 30 characters with ellipsis. This handles all long string values uniformly without needing to detect specific field types.
