Conversation
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.
Greptile SummaryThis PR fixes a one-line bug where Confidence Score: 5/5Safe 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: No files require special attention. Important Files Changed
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
Reviews (1): Last reviewed commit: "Use full_raw_path for heading anchor hre..." | Re-trigger Greptile |
| 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) |
There was a problem hiding this comment.
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.
| 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.
Summary
HeadingLinkbuilt its anchorhreffromrouter.page.full_path, the normalized (registered-route) path. Whenfrontend_pathis set, that path is stripped of the mount prefix, so the generatedhrefdrops 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 tofull_raw_path, which is the URL the client actually requested and preserves thefrontend_pathprefix.When
frontend_pathis unset,full_path == full_raw_path, so this is a no-op for the default case.The paired
rx.set_clipboard(href)inon_clickreads 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.pywhich walks the rendered component tree and asserts the href referencesrouter.page.full_raw_pathrather thanfull_path.