[REF-1993] link: respect is_external prop and other attributes on A…

… tag (reflex-dev#2651)

* link: respect `is_external` prop and other attributes on A tag

Instead of passing all props to NextLink by default, only pass props that
NextLink understands, placing the remaining props on the Radix link

Add a test case to avoid regression of `is_external` behavior.

* Link is a MemoizationLeaf

Because Link is often rendered with NextLink as_child, and NextLink breaks if
the href is stateful outside of a Link, ensure that any stateful child of Link
gets memoized together.

integration/test_navigation.py

@@ -0,0 +1,89 @@
+"""Integration tests for links and related components."""
+from typing import Generator
+from urllib.parse import urlsplit
+
+import pytest
+from selenium.webdriver.common.by import By
+
+from reflex.testing import AppHarness
+
+from .utils import poll_for_navigation
+
+
+def NavigationApp():
+    """Reflex app with links for navigation."""
+    import reflex as rx
+
+    class State(rx.State):
+        is_external: bool = True
+
+    app = rx.App()
+
+    @app.add_page
+    def index():
+        return rx.fragment(
+            rx.link("Internal", href="/internal", id="internal"),
+            rx.link(
+                "External",
+                href="/internal",
+                is_external=State.is_external,
+                id="external",
+            ),
+            rx.link(
+                "External Target", href="/internal", target="_blank", id="external2"
+            ),
+        )
+
+    @rx.page(route="/internal")
+    def internal():
+        return rx.text("Internal")
+
+
+@pytest.fixture()
+def navigation_app(tmp_path) -> Generator[AppHarness, None, None]:
+    """Start NavigationApp app at tmp_path via AppHarness.
+
+    Args:
+        tmp_path: pytest tmp_path fixture
+
+    Yields:
+        running AppHarness instance
+    """
+    with AppHarness.create(
+        root=tmp_path,
+        app_source=NavigationApp,  # type: ignore
+    ) as harness:
+        yield harness
+
+
+@pytest.mark.asyncio
+async def test_navigation_app(navigation_app: AppHarness):
+    """Type text after moving cursor. Update text on backend.
+
+    Args:
+        navigation_app: harness for NavigationApp app
+    """
+    assert navigation_app.app_instance is not None, "app is not running"
+    driver = navigation_app.frontend()
+
+    internal_link = driver.find_element(By.ID, "internal")
+
+    with poll_for_navigation(driver):
+        internal_link.click()
+    assert urlsplit(driver.current_url).path == f"/internal/"
+    with poll_for_navigation(driver):
+        driver.back()
+
+    external_link = driver.find_element(By.ID, "external")
+    external2_link = driver.find_element(By.ID, "external2")
+
+    external_link.click()
+    # Expect a new tab to open
+    assert AppHarness._poll_for(lambda: len(driver.window_handles) == 2)
+
+    # Switch back to the main tab
+    driver.switch_to.window(driver.window_handles[0])
+
+    external2_link.click()
+    # Expect another new tab to open
+    assert AppHarness._poll_for(lambda: len(driver.window_handles) == 3)

reflex/components/radix/themes/typography/link.py

@@ -6,7 +6,8 @@
 
 from typing import Literal
 
-from reflex.components.component import Component
+from reflex.components.component import Component, MemoizationLeaf
+from reflex.components.core.cond import cond
 from reflex.components.el.elements.inline import A
 from reflex.components.next.link import NextLink
 from reflex.utils import imports
@@ -27,7 +28,7 @@
 next_link = NextLink.create()
 
 
-class Link(RadixThemesComponent, A):
+class Link(RadixThemesComponent, A, MemoizationLeaf):
     """A semantic element for navigation between pages."""
 
     tag = "Link"
@@ -53,6 +54,9 @@ class Link(RadixThemesComponent, A):
     # Whether to render the text with higher contrast color
     high_contrast: Var[bool]
 
+    # If True, the link will open in a new tab
+    is_external: Var[bool]
+
     def _get_imports(self) -> imports.ImportDict:
         return {**super()._get_imports(), **next_link._get_imports()}
 
@@ -70,12 +74,23 @@ def create(cls, *children, **props) -> Component:
         Returns:
             Component: The link component
         """
+        is_external = props.pop("is_external", None)
+        if is_external is not None:
+            props["target"] = cond(is_external, "_blank", "")
         if props.get("href") is not None:
             if not len(children):
                 raise ValueError("Link without a child will not display")
             if "as_child" not in props:
+                # Extract props for the NextLink, the rest go to the Link/A element.
+                known_next_link_props = NextLink.get_props()
+                next_link_props = {}
+                for prop in props.copy():
+                    if prop in known_next_link_props:
+                        next_link_props[prop] = props.pop(prop)
                 # If user does not use `as_child`, by default we render using next_link to avoid page refresh during internal navigation
                 return super().create(
-                    NextLink.create(*children, **props), as_child=True
+                    NextLink.create(*children, **next_link_props),
+                    as_child=True,
+                    **props,
                 )
         return super().create(*children, **props)

reflex/components/radix/themes/typography/link.pyi

@@ -8,7 +8,8 @@ from reflex.vars import Var, BaseVar, ComputedVar
 from reflex.event import EventChain, EventHandler, EventSpec
 from reflex.style import Style
 from typing import Literal
-from reflex.components.component import Component
+from reflex.components.component import Component, MemoizationLeaf
+from reflex.components.core.cond import cond
 from reflex.components.el.elements.inline import A
 from reflex.components.next.link import NextLink
 from reflex.utils import imports
@@ -19,7 +20,7 @@ from .base import LiteralTextSize, LiteralTextTrim, LiteralTextWeight
 LiteralLinkUnderline = Literal["auto", "hover", "always"]
 next_link = NextLink.create()
 
-class Link(RadixThemesComponent, A):
+class Link(RadixThemesComponent, A, MemoizationLeaf):
     @overload
     @classmethod
     def create(  # type: ignore
@@ -113,6 +114,7 @@ class Link(RadixThemesComponent, A):
             ]
         ] = None,
         high_contrast: Optional[Union[Var[bool], bool]] = None,
+        is_external: Optional[Union[Var[bool], bool]] = None,
         download: Optional[
             Union[Var[Union[str, int, bool]], Union[str, int, bool]]
         ] = None,
@@ -238,6 +240,7 @@ class Link(RadixThemesComponent, A):
             underline: Sets the visibility of the underline affordance: "auto" | "hover" | "always"
             color_scheme: Overrides the accent color inherited from the Theme.
             high_contrast: Whether to render the text with higher contrast color
+            is_external: If True, the link will open in a new tab
             download: Specifies that the target (the file specified in the href attribute) will be downloaded when a user clicks on the hyperlink.
             href: Specifies the URL of the page the link goes to
             href_lang: Specifies the language of the linked document