Skip to content

feat(attributes): make URL attribute values clickable - #1660

Closed
giswqs wants to merge 1 commit into
mainfrom
fix/issue-1655-clickable-attribute-urls
Closed

feat(attributes): make URL attribute values clickable#1660
giswqs wants to merge 1 commit into
mainfrom
fix/issue-1655-clickable-attribute-urls

Conversation

@giswqs

@giswqs giswqs commented Aug 3, 2026

Copy link
Copy Markdown
Member

Problem

Reported in discussion #1655: a user put a web address into an attribute (a Link text field on a marker they drew). The address shows up in the marker's popup and in the attribute table, but it is plain text, so there is no way to follow it other than selecting the string and pasting it into a browser.

Nothing was linkifying attribute values. createIdentifyPopupElement in packages/map/src/MapCanvas.tsx set valueCell.textContent, and the attribute table cell rendered a bare formatAttributeValue(value) text node.

Change

  • packages/core/src/hyperlink.ts — new attributeLinkUrl(value). Deliberately strict: the whole trimmed value must be a single http(s) URL with an authority. Linkifying substrings of prose has to guess where a URL ends and gets trailing punctuation wrong, and a loose scheme test would let javascript: or file: reach an opener. mailto: is excluded too, since the opener cannot open it and a dead anchor is worse than text.
  • Identify popup and attribute table render such a value as an anchor. In the table the anchor is truncated inside its cell so a long URL cannot run over the neighbouring column and steal its clicks, with the full URL in title.
  • apps/geolibre-desktop/src/lib/external-link-interceptor.ts — new delegated click listener, installed once, desktop-only. The Tauri webview ignores target="_blank", so an untouched anchor there either does nothing or navigates the single app webview away from GeoLibre with no way back. Anchors are rendered in plenty of places outside React and outside this repo (Identify popups, KML <description> markup, plugin panels), so one document-level listener covers them all rather than patching call sites one at a time. It defers to same-origin links, non-http(s) schemes, modified and middle clicks, and any handler that already called preventDefault (the attribute table does).

This also fixes the anchors in KML <description> markup, which were built with target="_blank" and so were already dead clicks on the desktop build.

Verification

Driven in a real browser against the dev server.

  • The reporter's exact flow: GeoEditor marker → attribute table → Add field Link (text) with default https://www.bbc.co.uk/. The cell renders as a link, and clicking the marker with Identify shows one popup containing the same link.
  • USGS earthquake GeoJSON (2,062 features, each with a url and a detail property). Identify renders both as links; clicking one opens the real USGS event page. Confirmed the click reaches document with the anchor intact, which is what the desktop interceptor relies on.
  • Both light and dark themes.
  • npm run build, npm run test:frontend (4890 passing), npm run lint (0 errors), pre-commit run --files <changed>.

New tests: tests/attribute-hyperlink.test.ts (13 cases over attributeLinkUrl) and tests/external-link-interceptor.test.ts (7 cases over the interceptor).

Not covered

maplibre-gl-vector draws its own vector-control-popup for layers added through its panel, and that popup still renders URLs as text. It is a separate component in an upstream package, and it does not appear for the drawn-marker case in this report, so it is left for a follow-up.

Fixes #1655

Summary by CodeRabbit

  • New Features

    • Attribute values containing valid HTTP(S) addresses now appear as clickable links.
    • Links open securely in the system browser without selecting the surrounding map feature.
    • Long URLs wrap cleanly without expanding the Identify popup.
    • Internal and non-web values retain their existing display behavior.
  • Bug Fixes

    • Prevented unsafe, malformed, or unintended URLs from being treated as external links.
    • Preserved normal behavior for modified clicks and same-origin navigation.

An attribute whose value is a web address rendered as dead text in both
the Identify popup and the attribute table, so the only way to follow it
was to select the string and paste it into a browser. Detect a value that
is entirely an http(s) URL and render it as a real link in both places.

Adds `attributeLinkUrl` in @geolibre/core as the single, deliberately
strict test: the whole trimmed value must be one http(s) URL with an
authority. Linkifying substrings of prose would get trailing punctuation
wrong, and a loose scheme check would let javascript:/file: reach an
opener.

Also adds a delegated click interceptor for the desktop build. The Tauri
webview ignores target="_blank", so an anchor there either does nothing
or navigates the single app webview away from GeoLibre with no way back.
Anchors are rendered in plenty of places outside React and outside this
repo (Identify popups, KML <description> markup, plugin panels), so one
document-level listener handles them all rather than patching call sites
one at a time. It stays out of the way of same-origin links, non-http(s)
schemes, modified/middle clicks, and any handler that already claimed the
click.

Verified in the browser against the reporter's exact flow (a GeoEditor
marker with a "Link" text field) and against USGS earthquake GeoJSON,
whose features carry `url` and `detail`, in both light and dark themes.

Fixes #1655
Copilot AI review requested due to automatic review settings August 3, 2026 14:18
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change validates HTTP(S) attribute values, renders them as secure links in map and desktop attribute views, and opens eligible desktop links through the system browser. Tests cover URL validation and click interception.

Changes

Attribute hyperlink flow

Layer / File(s) Summary
URL contract and attribute rendering
packages/core/src/hyperlink.ts, packages/core/src/index.ts, packages/map/src/MapCanvas.tsx, tests/attribute-hyperlink.test.ts
attributeLinkUrl validates complete HTTP(S) URLs. Identify popup values render as secure links. Tests cover valid and rejected values.
Desktop attribute link activation
apps/geolibre-desktop/src/components/panels/AttributeTable.tsx, apps/geolibre-desktop/src/index.css
Desktop attribute cells render detected URLs with truncation and wrapping. Link clicks avoid row selection and call openExternalLink.
Desktop external-link interception
apps/geolibre-desktop/src/lib/external-link-interceptor.ts, apps/geolibre-desktop/src/main.tsx, tests/external-link-interceptor.test.ts
Tauri startup installs delegated interception for eligible outbound HTTP(S) clicks. Tests cover origin, scheme, modifier, mouse-button, and prevented-event handling.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: copilot

Poem

A rabbit spots a URL bright,
And wraps it neatly out of sight.
The desktop click hops through the gate,
The system browser opens straight.
Safe links now spring with cheer.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making URL attribute values clickable.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1655-clickable-attribute-urls

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Biome (2.5.5)
apps/geolibre-desktop/src/index.css

File contains syntax errors that prevent linting: Line 5: Tailwind-specific syntax is disabled.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://0535ff47.geolibre-preview.pages.dev
Demo app https://0535ff47.geolibre-preview.pages.dev/demo/
Commit bd163ae

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/geolibre-desktop/src/components/panels/AttributeTable.tsx`:
- Around line 1924-1946: Update the link onClick handler in AttributeTable so
every link click stops propagation to prevent row selection, but only unmodified
clicks call preventDefault() and openExternalLink(linkUrl). Preserve the
browser’s native behavior for Ctrl-, Cmd-, Shift-, and Alt-clicks by returning
without interception when modifier keys are present.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9a3980f2-a3e2-4aed-a5c5-3effcbec7ed6

📥 Commits

Reviewing files that changed from the base of the PR and between 24c8e20 and bd163ae.

📒 Files selected for processing (9)
  • apps/geolibre-desktop/src/components/panels/AttributeTable.tsx
  • apps/geolibre-desktop/src/index.css
  • apps/geolibre-desktop/src/lib/external-link-interceptor.ts
  • apps/geolibre-desktop/src/main.tsx
  • packages/core/src/hyperlink.ts
  • packages/core/src/index.ts
  • packages/map/src/MapCanvas.tsx
  • tests/attribute-hyperlink.test.ts
  • tests/external-link-interceptor.test.ts

Comment thread apps/geolibre-desktop/src/components/panels/AttributeTable.tsx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves UX around feature attributes by detecting when an attribute value is entirely an http(s) URL and rendering it as a clickable link in both the Identify popup (MapCanvas) and the Attribute Table. For the desktop (Tauri) build, it adds a document-level click interceptor to safely route outbound link clicks to the system browser, avoiding the webview’s _blank limitations.

Changes:

  • Add attributeLinkUrl() to @geolibre/core to strictly recognize whole-value http(s) URLs (rejecting unsafe/non-openable schemes and prose substrings).
  • Render URL-valued attributes as <a> elements in the Identify popup and Attribute Table (with truncation + title in the table).
  • Add a desktop-only, delegated external link interceptor (plus CSS styling and targeted tests).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/external-link-interceptor.test.ts Adds coverage for the delegated Tauri-only click interceptor behavior.
tests/attribute-hyperlink.test.ts Adds unit tests for strict URL detection in attributeLinkUrl().
packages/map/src/MapCanvas.tsx Linkifies URL-valued attribute entries in Identify popups.
packages/core/src/index.ts Exports the new hyperlink helper from @geolibre/core.
packages/core/src/hyperlink.ts Introduces strict URL detection for attribute values.
apps/geolibre-desktop/src/main.tsx Installs the external link interceptor at startup (desktop-only behavior gated at runtime).
apps/geolibre-desktop/src/lib/external-link-interceptor.ts Implements delegated outbound http(s) link interception for Tauri.
apps/geolibre-desktop/src/index.css Styles Identify-popup attribute links to match existing KML anchor styling and wrap long URLs.
apps/geolibre-desktop/src/components/panels/AttributeTable.tsx Renders URL-only attribute cells as clickable links and opens them via openExternalLink.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code review

I read through the full diff (hyperlink.ts, the MapCanvas.tsx and AttributeTable.tsx render paths, external-link-interceptor.ts, main.tsx, the CSS addition, and both new test files) and cross-checked the surrounding source (open-external.ts, is-tauri.ts/tauri-io.ts, the TableCell/column-width logic, existing .geolibre-kml-description a styling) to verify the changed lines are correct in context.

Bugs: None found. attributeLinkUrl's whitespace/scheme/authority checks hold up against the adversarial cases I traced by hand (case-insensitive scheme, empty-authority shapes like https://:8080/x, zero-width-space smuggling, javascript:/file:/data: schemes, multi-URL prose) — all fall through correctly to null or a safe host. The interceptor's sameOrigin, modifier-key, and defaultPrevented/stopPropagation bubble-order logic (attribute table's own handler wins over the document-level interceptor) checked out against the test suite's stated intent. Table-cell truncation is backed by fixed <col style={{width}}> widths, so the truncate/max-w-full anchor styling will actually clip rather than blow out the column. (Confidence: high — no functional bug identified.)

Security: None found. New anchors use property assignment / JSX props (href, textContent), never innerHTML, so no injection vector. The scheme allowlist is enforced twice independently (attributeLinkUrl and openExternalLink), and rel="noopener noreferrer" is set everywhere an anchor is created. mailto:/javascript:/file:/data: are all correctly excluded from the openable set. (Confidence: high.)

Performance: Minor, low-severity nit: attributeLinkUrl(value) in AttributeTable.tsx is computed for every rendered cell on every render, including cells currently in edit mode where the result is unused. Given the table is virtualized (only visible rows render) and the check is a cheap regex plus a new URL() call, this is very unlikely to matter in practice. (Confidence: low.)

Quality: The identify popup (MapCanvas.tsx, in the shared @geolibre/map package) relies purely on native target="_blank" plus the desktop-only document-level interceptor, while the attribute table (app-level code) calls openExternalLink directly with its own preventDefault/stopPropagation. This is a deliberate and reasonable split (the map package can't depend on app-specific Tauri plumbing), not a bug, but worth flagging as an intentional asymmetry for future maintainers rather than an oversight. (Confidence: low, informational only.)

CLAUDE.md adherence: No new user-facing strings were introduced (the only new visible text is the URL itself, used as title/content), so no i18n catalog update was needed. No physical-direction Tailwind utilities were added, keeping RTL compatibility intact. New/updated exports from @geolibre/core follow the existing barrel-export pattern. Nothing here required a pyproject.toml/lockfile, WASM-catalog, or CSP-allowlist update. (Confidence: high.)

Overall this is a well-scoped, carefully tested change (13 + 7 new unit test cases covering the strict-URL parser and the interceptor's edge cases) with no significant issues identified.

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.

2 participants