Skip to content

Use full_raw_path for heading anchor hrefs#6341

Open
opadron wants to merge 1 commit intomainfrom
claude/fix/heading-full-raw-path
Open

Use full_raw_path for heading anchor hrefs#6341
opadron wants to merge 1 commit intomainfrom
claude/fix/heading-full-raw-path

Conversation

@opadron
Copy link
Copy Markdown

@opadron opadron commented Apr 18, 2026

Summary

HeadingLink built its anchor href from router.page.full_path, the normalized (registered-route) path. When frontend_path is set, that path is stripped of the mount prefix, so the generated href drops the prefix too — clicking jumps to a route that doesn't exist on the ingress, and anchor-only navigation on the same page lands on a 404. Switch to full_raw_path, which is the URL the client actually requested and preserves the frontend_path prefix.

When frontend_path is unset, full_path == full_raw_path, so this is a no-op for the default case.

The paired rx.set_clipboard(href) in on_click reads the same local, so the "copy link" action and the rendered href stay consistent.

Regression test

Added tests/units/reflex_ui_shared/components/blocks/test_headings.py which walks the rendered component tree and asserts the href references router.page.full_raw_path rather than full_path.

HeadingLink built its anchor href from router.page.full_path, which is the
normalized (registered-route) path. When frontend_path is set, that path is
stripped of the mount prefix; the generated href drops the prefix too and
clicking jumps to a route that doesn't exist on the ingress. Switch to
full_raw_path, which is the URL the client actually requested and preserves
the frontend_path prefix. When frontend_path is unset, full_path equals
full_raw_path, so this is a no-op for that case.

The paired rx.set_clipboard(href) in on_click reads the same local, so the
"copy link" action and the rendered href stay consistent.
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 18, 2026

Merging this PR will not alter performance

✅ 9 untouched benchmarks


Comparing claude/fix/heading-full-raw-path (cd0a868) with main (e3c4c29)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 18, 2026

Greptile Summary

This PR fixes a one-line bug where HeadingLink built its anchor href from router.page.full_path (the normalized registered route) instead of router.page.full_raw_path (the URL the client actually requested). When frontend_path is configured, full_path strips the mount prefix, so anchor links and the "copy link" clipboard action would reference a non-existent route at the ingress. Since full_path == full_raw_path when frontend_path is not set, this change is a no-op for the default deployment case. A regression test is included that walks the rendered component tree and asserts full_raw_path appears in the props.

Confidence Score: 5/5

Safe to merge — the fix is a correct one-line change and the only remaining finding is a minor P2 style issue in the new test.

No P0 or P1 issues found. The single-line change is clearly correct: full_raw_path is an existing router field, and full_path == full_raw_path in the default case, making this a no-op for existing deployments. The regression test follows established patterns in the codebase (_render().props is already used in test_component.py). The only finding is a missing type annotation on the helper function, which is P2.

No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-ui-shared/src/reflex_ui_shared/components/blocks/headings.py Single-line fix replacing full_path with full_raw_path in the href used by heading anchors and the clipboard copy-link handler.
tests/units/reflex_ui_shared/components/blocks/test_headings.py New regression test asserting the rendered props contain full_raw_path and not full_path; the component parameter in the helper lacks a type annotation.
tests/units/reflex_ui_shared/init.py Empty __init__.py to register the new test package directory.
tests/units/reflex_ui_shared/components/init.py Empty __init__.py to register the new test sub-package directory.
tests/units/reflex_ui_shared/components/blocks/init.py Empty __init__.py to register the new test sub-package directory.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["HeadingLink.create(text, heading)"] --> B["Build anchor href"]
    B --> C{"frontend_path\nconfigured?"}
    C -->|No| D["full_path == full_raw_path\n(no difference)"]
    C -->|Yes| E_old["❌ OLD: full_path\nstrips mount prefix"]
    C -->|Yes| E_new["✅ NEW: full_raw_path\npreserves mount prefix"]
    D --> F["Correct anchor href\n+ clipboard copy"]
    E_old --> G["Broken href → 404 at ingress"]
    E_new --> F
Loading

Reviews (1): Last reviewed commit: "Use full_raw_path for heading anchor hre..." | Re-trigger Greptile

Comment on lines +6 to +19
def _all_rendered_prop_text(component) -> str:
"""Collect the string form of every prop across a component tree.

Args:
component: A Reflex component.

Returns:
A single string containing the concatenated props of the component and
all of its descendants.
"""
texts = [str(component._render().props)]
for child in component.children:
texts.append(_all_rendered_prop_text(child))
return " ".join(texts)
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.

P2 Missing type annotation on component parameter

_all_rendered_prop_text is untyped on its parameter, which pyright will flag and which doesn't match the Google-style docstring convention used elsewhere in the codebase. Adding a type makes it easier to understand what the helper accepts.

Suggested change
def _all_rendered_prop_text(component) -> str:
"""Collect the string form of every prop across a component tree.
Args:
component: A Reflex component.
Returns:
A single string containing the concatenated props of the component and
all of its descendants.
"""
texts = [str(component._render().props)]
for child in component.children:
texts.append(_all_rendered_prop_text(child))
return " ".join(texts)
def _all_rendered_prop_text(component: rx.Component) -> str:

You'll also need import reflex as rx at the top alongside the existing import.

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.

1 participant