[Feature]- Option to open Quicklinks in same page (Issue #8621)#8734
[Feature]- Option to open Quicklinks in same page (Issue #8621)#8734Ivanq50 wants to merge 5 commits intomakeplane:previewfrom
Conversation
…e functionality of opening quicklinks in same page. Can toggle the preference (same page or new tab) with a button click and can also choose individually by link.
📝 WalkthroughWalkthroughThis pull request adds manual page reordering and quick links features. Pages now include a Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Page List UI
participant Store as ProjectPageStore
participant API as Backend API
participant DB as Database
User->>UI: Drag and drop page
activate UI
UI->>UI: Calculate drop position (above/below)
UI->>Store: handlePageDrop(sourceId, destId, edge)
deactivate UI
activate Store
Store->>Store: Validate prerequisites
Store->>Store: Calculate new sort_order values
Store->>Store: Optimistically update local state
Store->>API: reorderPage(workspaceSlug, projectId, pageId, destPageId, edge)
deactivate Store
activate API
API->>DB: Update sort_order values
DB-->>API: Confirmed
API-->>Store: Success response
deactivate API
activate Store
Store->>Store: Confirm local state
Store->>UI: Notify observers of update
deactivate Store
activate UI
UI->>User: Reordered list displayed
deactivate UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
|
|
There was a problem hiding this comment.
Pull request overview
Adds a user preference to open Quick Links in the same tab, and also introduces a manual ordering mode for Pages backed by a new sort_order field across the stack.
Changes:
- Add a Quick Links “open in same tab” preference (toggle + context menu action) persisted in localStorage.
- Add Page manual ordering via
sort_order(API ordering + serializer field + web drag/drop reorder + new sort option). - Normalize base path handling for admin/space builds and add/adjust API contract tests & test settings.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/page.ts | Adds sort_order ordering support and switches to toReversed() for desc sorting. |
| packages/types/src/page/core.ts | Extends TPage with sort_order and adds sort_order as a valid sort key. |
| packages/i18n/src/locales/en/translations.ts | Adds new Quick Links strings for same/new tab options. |
| apps/web/core/store/pages/project-page.store.ts | Defaults page sorting to sort_order and adds a reorderPage action that patches sort_order. |
| apps/web/core/store/pages/base-page.ts | Stores/serializes sort_order on the base page model. |
| apps/web/core/components/pages/list/root.tsx | Enables page reordering in the list and surfaces reorder failures via toast. |
| apps/web/core/components/pages/list/order-by.tsx | Adds “Manual order” to the page sort dropdown. |
| apps/web/core/components/pages/list/block.tsx | Adds pragmatic-dnd drag/drop behavior for list items + drag handle UI. |
| apps/web/core/components/home/widgets/links/root.tsx | Adds Quick Links header toggle to switch open-in-same-tab preference. |
| apps/web/core/components/home/widgets/links/link-detail.tsx | Implements primary click behavior based on preference + adds “Open in same tab” menu option. |
| apps/web/core/components/home/widgets/links/constants.ts | Defines the localStorage key and default for the Quick Links preference. |
| apps/space/vite.config.ts | Replaces joinUrlPath with a local normalizeBasePath helper for Vite base. |
| apps/space/react-router.config.ts | Mirrors base path normalization for react-router config. |
| apps/admin/vite.config.ts | Replaces joinUrlPath with normalizeBasePath for admin Vite base. |
| apps/admin/react-router.config.ts | Mirrors base path normalization for admin react-router config. |
| apps/api/plane/app/views/page/base.py | Orders pages by -sort_order and allows sort_order patching even when locked. |
| apps/api/plane/app/serializers/page.py | Adds sort_order to page serializer fields. |
| apps/api/plane/tests/contract/app/test_quick_links.py | Adds contract tests for Quick Links CRUD endpoints. |
| apps/api/plane/tests/contract/app/test_pages_app.py | Adds contract tests for page list ordering and sort_order patching/locking rules. |
| apps/api/plane/settings/test.py | Removes WhiteNoise middleware in tests to avoid static-root warnings. |
| apps/api/plane/settings/common.py | Removes deprecated USE_L10N. |
| apps/api/plane/celery.py | Changes JsonFormatter import path for Celery logging. |
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | ||
| }; |
There was a problem hiding this comment.
normalizeBasePath is duplicated across multiple config files (admin + space, vite + react-router). This increases the chance of the base-path normalization logic drifting over time. Consider extracting it into a small shared helper (e.g., in @plane/utils or a local config-utils module) and importing it from each config file.
| const handlePageDrop = async ( | ||
| sourcePageId: string, | ||
| destinationPageId: string, | ||
| edge: "reorder-above" | "reorder-below" | ||
| ) => { | ||
| if (!workspaceSlug || !projectId || sourcePageId === destinationPageId || !filteredPageIds?.length) return; | ||
| try { | ||
| await reorderPage( | ||
| workspaceSlug.toString(), | ||
| projectId.toString(), | ||
| sourcePageId, | ||
| destinationPageId, | ||
| edge, | ||
| filteredPageIds | ||
| ); | ||
| } catch { | ||
| setToast({ | ||
| type: TOAST_TYPE.ERROR, | ||
| title: t("toast.error"), | ||
| message: "Failed to reorder page. Please try again.", | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
handlePageDrop is recreated on every render and is passed into PageListBlock. Since PageListBlock's drag/drop useEffect depends on onPageDrop, this will cause the drag/drop adapters to be torn down and re-registered every render, which can lead to flaky DnD behavior and unnecessary work. Wrap handlePageDrop in useCallback (with the needed dependencies) or otherwise ensure a stable callback reference is passed to PageListBlock.
| open_in_same_tab: "Open in same tab", | ||
| open_links_in_new_tab: "Open links in new tab", | ||
| open_links_in_same_tab: "Open links in same tab", |
There was a problem hiding this comment.
New quick-links translation keys (open_in_same_tab, open_links_in_new_tab, open_links_in_same_tab) were added only to the English locale. Other locale translation files under packages/i18n/src/locales/*/translations.ts do not define these keys, which will cause missing-string fallbacks/inconsistent UX for non-English users. Please add the same keys to the other locales (even if temporarily copying the English text) or update the i18n loading strategy to guarantee an explicit fallback for these keys.
| open_in_same_tab: "Open in same tab", | |
| open_links_in_new_tab: "Open links in new tab", | |
| open_links_in_same_tab: "Open links in same tab", |
| # Third party imports | ||
| from celery import Celery | ||
| from pythonjsonlogger.jsonlogger import JsonFormatter | ||
| from pythonjsonlogger.json import JsonFormatter |
There was a problem hiding this comment.
python-json-logger is pinned to python-json-logger==3.3.0 (apps/api/requirements/base.txt) and the rest of the codebase references pythonjsonlogger.jsonlogger.JsonFormatter in LOGGING config. Importing JsonFormatter from pythonjsonlogger.json here is inconsistent and is likely to raise ModuleNotFoundError at worker startup. Please switch this import back to from pythonjsonlogger.jsonlogger import JsonFormatter (or update dependencies/config consistently if the new module path is intentional).
| from pythonjsonlogger.json import JsonFormatter | |
| from pythonjsonlogger.jsonlogger import JsonFormatter |
| filters: TPageFilters = { | ||
| searchQuery: "", | ||
| sortKey: "updated_at", | ||
| sortKey: "sort_order", | ||
| sortBy: "desc", | ||
| }; |
There was a problem hiding this comment.
This PR is described as a Quick Links UX feature (open in same tab), but it also introduces a separate Pages feature (manual sorting/reordering via sort_order) and changes unrelated build/runtime configuration (admin/space base path normalization, logging import changes, Django settings tweaks). Please update the PR title/description to reflect the full scope or split these into separate PRs so reviewers can reason about risk and intent more clearly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/pages/list/block.tsx (1)
50-54:⚠️ Potential issue | 🔴 CriticalHooks called after conditional return violates React's Rules of Hooks.
The
useEffecton line 54 is placed after the early return on line 50. React requires hooks to be called in the same order on every render. Ifpageisnull, the hook won't be called, causing inconsistent hook ordering.🐛 Proposed fix: Move early return after all hooks
const { isMobile } = usePlatformOS(); - // handle page check - if (!page) return null; - // derived values - const { name, logo_props, getRedirectionLink } = page; useEffect(() => { const rowElement = parentRef.current; - if (!rowElement || !isReorderEnabled) return; + if (!rowElement || !isReorderEnabled || !page) return; const initialData = { id: pageId, dragInstanceId: "PROJECT_PAGES" }; return combine( // ... rest of effect ); }, [isReorderEnabled, isLastChild, onPageDrop, pageId]); + // handle page check + if (!page) return null; + // derived values + const { name, logo_props, getRedirectionLink } = page; + return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/pages/list/block.tsx` around lines 50 - 54, The early return "if (!page) return null;" is placed before hooks which breaks Rules of Hooks; move that conditional return so all hooks (e.g., the useEffect starting at the current line 54) are declared before any early returns, or alternatively change the early-return to a conditional render inside the JSX while keeping the useEffect and any other hooks (referencing page, name, logo_props, getRedirectionLink) at the top of the component; ensure hooks are always executed in the same order regardless of page being null.
🧹 Nitpick comments (3)
apps/api/plane/tests/contract/app/test_quick_links.py (1)
115-123: Moveuuidimport to the top of the file.The
uuidimport on line 118 should be moved to the module level with other imports for consistency.♻️ Proposed fix
Add to imports at top of file:
from unittest.mock import patch +import uuid + import pytest from rest_framework import statusThen update the test:
def test_retrieve_nonexistent_returns_404(self, session_client, workspace): """Retrieve with invalid uuid returns 404.""" - import uuid - fake_id = uuid.uuid4()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/plane/tests/contract/app/test_quick_links.py` around lines 115 - 123, Move the local import of uuid from inside test_retrieve_nonexistent_returns_404 to the module-level imports at the top of the test file so all imports are consistent; update the top import block to include uuid and remove the inline `import uuid` inside the test that uses fake_id and _quick_links_detail_url(session_client, workspace) so the test simply calls uuid.uuid4() without an inline import.apps/admin/vite.config.ts (1)
7-23: ExtractnormalizeBasePath()to a shared config helper.This logic is now duplicated in
apps/admin/vite.config.ts,apps/space/vite.config.ts,apps/admin/react-router.config.ts, andapps/space/react-router.config.ts. Centralizing it will keep Vitebaseand routerbasenamefrom drifting the next time the normalization rules change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/vite.config.ts` around lines 7 - 23, The normalizeBasePath function is duplicated; extract it into a shared config helper module, export normalizeBasePath, and replace the local definitions in files that use it (references: normalizeBasePath, VITE_ADMIN_BASE_PATH, viteEnv) with imports from that helper; ensure the helper accepts a string and returns the same trimmed/normalized output and update each vite.config.ts and react-router.config.ts to import and use the shared normalizeBasePath so both Vite `base` and router `basename` remain consistent.apps/web/core/components/pages/list/root.tsx (1)
58-64: Consider translating the error message for consistency.The toast title uses
t("toast.error")but the message is hardcoded in English. For i18n consistency, the message should also be translated.♻️ Suggested improvement
} catch { setToast({ type: TOAST_TYPE.ERROR, title: t("toast.error"), - message: "Failed to reorder page. Please try again.", + message: t("pages.reorder_error"), }); }This requires adding the corresponding translation key to the i18n catalog.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/core/components/pages/list/root.tsx` around lines 58 - 64, The toast error message is hardcoded in English; update the catch block that calls setToast with TOAST_TYPE.ERROR to use the i18n helper t(...) for the message (e.g., replace the literal "Failed to reorder page. Please try again." with a translation key like t("toast.reorderPageFailed")); also add the corresponding translation entry to the i18n catalog for that key in all locales so the message is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/plane/app/views/page/base.py`:
- Around line 103-105: The current queryset first applies the caller's ?order_by
then unconditionally calls .order_by("-is_favorite", "-sort_order",
"-created_at"), which overwrites the explicit ordering; change the logic in the
view (around the queryset building that calls self.request.GET.get("order_by",
"-created_at") and the subsequent .order_by("-is_favorite", "-sort_order",
"-created_at")) so the latter default ordering is only applied when no order_by
parameter was provided (e.g., check if self.request.GET.get("order_by") is falsy
and only then call .order_by("-is_favorite", "-sort_order", "-created_at")).
In `@apps/space/vite.config.ts`:
- Around line 7-10: normalizeBasePath currently only trims leading/trailing
slashes so inputs like "space//preview" become "/space//preview/"; update the
normalizeBasePath function to also collapse repeated slashes inside the path by
replacing consecutive "/" groups with a single "/" (e.g., use a replace like
value.replace(/\/{2,}/g, "/")) after trimming and before adding the
leading/trailing slash, and keep the function signature and return behavior the
same.
In `@apps/web/core/components/home/widgets/links/root.tsx`:
- Around line 88-90: The add-trigger button in the Links widget (the button
using handleCreateLinkModal and rendering PlusIcon in root.tsx) lacks an
explicit type and will default to submit inside a form; update that button
element to include type="button" so clicking it won't submit any parent form,
leaving onClick={handleCreateLinkModal} and className intact.
- Around line 71-86: The button rendering the toggle lacks an accessible pressed
state; update the button element (the one using handleToggleOpenInSameTab and
reading openInSameTab/DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) to include an
aria-pressed attribute that reflects the current boolean state ((openInSameTab
?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB)), so assistive tech can determine
on/off while keeping the existing aria-label and click handler intact.
---
Outside diff comments:
In `@apps/web/core/components/pages/list/block.tsx`:
- Around line 50-54: The early return "if (!page) return null;" is placed before
hooks which breaks Rules of Hooks; move that conditional return so all hooks
(e.g., the useEffect starting at the current line 54) are declared before any
early returns, or alternatively change the early-return to a conditional render
inside the JSX while keeping the useEffect and any other hooks (referencing
page, name, logo_props, getRedirectionLink) at the top of the component; ensure
hooks are always executed in the same order regardless of page being null.
---
Nitpick comments:
In `@apps/admin/vite.config.ts`:
- Around line 7-23: The normalizeBasePath function is duplicated; extract it
into a shared config helper module, export normalizeBasePath, and replace the
local definitions in files that use it (references: normalizeBasePath,
VITE_ADMIN_BASE_PATH, viteEnv) with imports from that helper; ensure the helper
accepts a string and returns the same trimmed/normalized output and update each
vite.config.ts and react-router.config.ts to import and use the shared
normalizeBasePath so both Vite `base` and router `basename` remain consistent.
In `@apps/api/plane/tests/contract/app/test_quick_links.py`:
- Around line 115-123: Move the local import of uuid from inside
test_retrieve_nonexistent_returns_404 to the module-level imports at the top of
the test file so all imports are consistent; update the top import block to
include uuid and remove the inline `import uuid` inside the test that uses
fake_id and _quick_links_detail_url(session_client, workspace) so the test
simply calls uuid.uuid4() without an inline import.
In `@apps/web/core/components/pages/list/root.tsx`:
- Around line 58-64: The toast error message is hardcoded in English; update the
catch block that calls setToast with TOAST_TYPE.ERROR to use the i18n helper
t(...) for the message (e.g., replace the literal "Failed to reorder page.
Please try again." with a translation key like t("toast.reorderPageFailed"));
also add the corresponding translation entry to the i18n catalog for that key in
all locales so the message is available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f45d0db-059c-4b94-8a0a-fb688244daa7
📒 Files selected for processing (22)
apps/admin/react-router.config.tsapps/admin/vite.config.tsapps/api/plane/app/serializers/page.pyapps/api/plane/app/views/page/base.pyapps/api/plane/celery.pyapps/api/plane/settings/common.pyapps/api/plane/settings/test.pyapps/api/plane/tests/contract/app/test_pages_app.pyapps/api/plane/tests/contract/app/test_quick_links.pyapps/space/react-router.config.tsapps/space/vite.config.tsapps/web/core/components/home/widgets/links/constants.tsapps/web/core/components/home/widgets/links/link-detail.tsxapps/web/core/components/home/widgets/links/root.tsxapps/web/core/components/pages/list/block.tsxapps/web/core/components/pages/list/order-by.tsxapps/web/core/components/pages/list/root.tsxapps/web/core/store/pages/base-page.tsapps/web/core/store/pages/project-page.store.tspackages/i18n/src/locales/en/translations.tspackages/types/src/page/core.tspackages/utils/src/page.ts
💤 Files with no reviewable changes (1)
- apps/api/plane/settings/common.py
| .order_by(self.request.GET.get("order_by", "-created_at")) | ||
| .prefetch_related("labels") | ||
| .order_by("-is_favorite", "-created_at") | ||
| .order_by("-is_favorite", "-sort_order", "-created_at") |
There was a problem hiding this comment.
Preserve explicit order_by requests here.
Line 103 already applies the caller's ?order_by=..., but Line 105 replaces it unconditionally, so every list request now falls back to -is_favorite, -sort_order, -created_at. If sort_order is only meant to be the default, gate this second ordering behind the absence of order_by instead of clobbering it.
Suggested fix
+ requested_order = self.request.GET.get("order_by")
+
return self.filter_queryset(
super()
.get_queryset()
.filter(workspace__slug=self.kwargs.get("slug"))
@@
.select_related("workspace")
.select_related("owned_by")
.annotate(is_favorite=Exists(subquery))
- .order_by(self.request.GET.get("order_by", "-created_at"))
.prefetch_related("labels")
- .order_by("-is_favorite", "-sort_order", "-created_at")
+ .order_by(
+ *(
+ ["-is_favorite", requested_order]
+ if requested_order
+ else ["-is_favorite", "-sort_order", "-created_at"]
+ )
+ )
.annotate(
project=Exists(
ProjectPage.objects.filter(page_id=OuterRef("id"), project_id=self.kwargs.get("project_id"))
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/plane/app/views/page/base.py` around lines 103 - 105, The current
queryset first applies the caller's ?order_by then unconditionally calls
.order_by("-is_favorite", "-sort_order", "-created_at"), which overwrites the
explicit ordering; change the logic in the view (around the queryset building
that calls self.request.GET.get("order_by", "-created_at") and the subsequent
.order_by("-is_favorite", "-sort_order", "-created_at")) so the latter default
ordering is only applied when no order_by parameter was provided (e.g., check if
self.request.GET.get("order_by") is falsy and only then call
.order_by("-is_favorite", "-sort_order", "-created_at")).
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; |
There was a problem hiding this comment.
Collapse repeated / characters inside the base path.
This only strips slashes at the ends, so VITE_SPACE_BASE_PATH="space//preview" normalizes to /space//preview/. That can bleed into generated asset URLs, and the same helper is copied into the admin/router configs too.
Suggested fix
const normalizeBasePath = (value: string): string => {
const trimmed = value.trim();
if (!trimmed || trimmed === "/") return "/";
- return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`;
+ const normalized = trimmed.replace(/\/{2,}/g, "/").replace(/^\/+|\/+$/g, "");
+ return normalized ? `/${normalized}/` : "/";
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const normalizeBasePath = (value: string): string => { | |
| const trimmed = value.trim(); | |
| if (!trimmed || trimmed === "/") return "/"; | |
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | |
| const normalizeBasePath = (value: string): string => { | |
| const trimmed = value.trim(); | |
| if (!trimmed || trimmed === "/") return "/"; | |
| const normalized = trimmed.replace(/\/{2,}/g, "/").replace(/^\/+|\/+$/g, ""); | |
| return normalized ? `/${normalized}/` : "/"; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/space/vite.config.ts` around lines 7 - 10, normalizeBasePath currently
only trims leading/trailing slashes so inputs like "space//preview" become
"/space//preview/"; update the normalizeBasePath function to also collapse
repeated slashes inside the path by replacing consecutive "/" groups with a
single "/" (e.g., use a replace like value.replace(/\/{2,}/g, "/")) after
trimming and before adding the leading/trailing slash, and keep the function
signature and return behavior the same.
| <button | ||
| type="button" | ||
| onClick={handleToggleOpenInSameTab} | ||
| className="rounded p-1 text-tertiary transition-colors hover:bg-surface-2 hover:text-primary" | ||
| aria-label={ | ||
| (openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) | ||
| ? t("home.quick_links.open_links_in_same_tab") | ||
| : t("home.quick_links.open_links_in_new_tab") | ||
| } | ||
| > | ||
| {(openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) ? ( | ||
| <LinkIcon className="size-4" /> | ||
| ) : ( | ||
| <NewTabIcon className="size-4" /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Expose the toggle state with aria-pressed.
Right now assistive tech gets a label, but not the on/off state of this toggle. Adding aria-pressed makes the current mode explicit.
♿ Suggested tweak
<button
type="button"
onClick={handleToggleOpenInSameTab}
className="rounded p-1 text-tertiary transition-colors hover:bg-surface-2 hover:text-primary"
+ aria-pressed={openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB}
aria-label={
(openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB)
? t("home.quick_links.open_links_in_same_tab")
: t("home.quick_links.open_links_in_new_tab")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| onClick={handleToggleOpenInSameTab} | |
| className="rounded p-1 text-tertiary transition-colors hover:bg-surface-2 hover:text-primary" | |
| aria-label={ | |
| (openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) | |
| ? t("home.quick_links.open_links_in_same_tab") | |
| : t("home.quick_links.open_links_in_new_tab") | |
| } | |
| > | |
| {(openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) ? ( | |
| <LinkIcon className="size-4" /> | |
| ) : ( | |
| <NewTabIcon className="size-4" /> | |
| )} | |
| </button> | |
| <button | |
| type="button" | |
| onClick={handleToggleOpenInSameTab} | |
| className="rounded p-1 text-tertiary transition-colors hover:bg-surface-2 hover:text-primary" | |
| aria-pressed={openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB} | |
| aria-label={ | |
| (openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) | |
| ? t("home.quick_links.open_links_in_same_tab") | |
| : t("home.quick_links.open_links_in_new_tab") | |
| } | |
| > | |
| {(openInSameTab ?? DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) ? ( | |
| <LinkIcon className="size-4" /> | |
| ) : ( | |
| <NewTabIcon className="size-4" /> | |
| )} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/core/components/home/widgets/links/root.tsx` around lines 71 - 86,
The button rendering the toggle lacks an accessible pressed state; update the
button element (the one using handleToggleOpenInSameTab and reading
openInSameTab/DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB) to include an aria-pressed
attribute that reflects the current boolean state ((openInSameTab ??
DEFAULT_QUICK_LINKS_OPEN_IN_SAME_TAB)), so assistive tech can determine on/off
while keeping the existing aria-label and click handler intact.
| <button onClick={handleCreateLinkModal} className="flex gap-1 text-13 font-medium text-accent-primary"> | ||
| <PlusIcon className="my-auto size-4" /> <span>{t("home.quick_links.add")}</span> | ||
| </button> |
There was a problem hiding this comment.
Set an explicit button type on the add trigger.
This still defaults to submit, so embedding the widget inside a form would submit the parent form unexpectedly.
🔧 Suggested tweak
- <button onClick={handleCreateLinkModal} className="flex gap-1 text-13 font-medium text-accent-primary">
+ <button
+ type="button"
+ onClick={handleCreateLinkModal}
+ className="flex gap-1 text-13 font-medium text-accent-primary"
+ >
<PlusIcon className="my-auto size-4" /> <span>{t("home.quick_links.add")}</span>
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button onClick={handleCreateLinkModal} className="flex gap-1 text-13 font-medium text-accent-primary"> | |
| <PlusIcon className="my-auto size-4" /> <span>{t("home.quick_links.add")}</span> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={handleCreateLinkModal} | |
| className="flex gap-1 text-13 font-medium text-accent-primary" | |
| > | |
| <PlusIcon className="my-auto size-4" /> <span>{t("home.quick_links.add")}</span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/core/components/home/widgets/links/root.tsx` around lines 88 - 90,
The add-trigger button in the Links widget (the button using
handleCreateLinkModal and rendering PlusIcon in root.tsx) lacks an explicit type
and will default to submit inside a form; update that button element to include
type="button" so clicking it won't submit any parent form, leaving
onClick={handleCreateLinkModal} and className intact.
Description
Added an option for the user to open links within the same page if desired. Placed a toggle button to set that preference, but the user also has the option to select that preference individually by clicking the "Open in new tab" or "Open in same tab" option after clicking the more options menu next to the link.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
Tested using unit tests which included:
Confirming the list endpoints behave consistently when there is no data.
Confirming both the API response contract and the database side effect.
Confirming the title is truly optional and the minimal valid payload works.
Confirming server-side validation for the url field is enforced.
Confirming url is required and missing required fields are rejected.
Confirming the detail endpoint can fetch a created resource and that the returned representation is correct.
Check that partial updates are working correctly and don't overwrite unspecified fields.
Confirm the delete behavior and ensures the test environment stays deterministic.
Confirm the API returns the correct status code for missing resources.
References
https://github.com/makeplane/plane/issues/8621
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores