opensource/dify
2
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-05-13 08:57:28 +08:00
Code Issues Packages Projects Releases Wiki Activity

docs: update frontend review guidance

Browse Source 
Document shared component reuse and component-writing checks for future frontend reviews, and refresh the MainNav follow-up notes.
This commit is contained in:
Jingyi-Dify 2026-05-12 16:04:44 -07:00
parent d076fc35b7
commit 39c448a6ca
3 changed files with 65 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
.agents/skills/frontend-code-review/SKILL.md
View File

@ -1,6 +1,6 @@
---
name: frontend-code-review
description: "Trigger when the user requests a review of frontend files (e.g., `.tsx`, `.ts`, `.js`). Support both pending-change reviews and focused file reviews while applying the checklist rules."
description: "Trigger when the user requests a review of frontend files (e.g., `.tsx`, `.ts`, `.js`). Support pending-change and focused file reviews while applying checklist rules, shared component reuse checks, and React component structure guidance from how-to-write-component."
---
# Frontend Code Review
@ -16,10 +16,12 @@ Stick to the checklist below for every applicable file and mode.
## Checklist
See [references/code-quality.md](references/code-quality.md), [references/performance.md](references/performance.md), [references/business-logic.md](references/business-logic.md) for the living checklist split by category—treat it as the canonical set of rules to follow.
When reviewing React/TypeScript components, also apply the repo-local `how-to-write-component` skill as the component architecture checklist. In particular, check ownership boundaries, props and API types, query/mutation usage, navigation choices, effect usage, unnecessary wrappers, and unnecessary memoization.
Flag each rule violation with urgency metadata so future reviewers can prioritize fixes.
## Review Process
1. Open the relevant component/module. Gather lines that relate to class names, styling/CSS imports, file size and component boundaries, i18n keys, behavior-sensitive UI interactions, React Flow hooks, and prop memoization.
1. Open the relevant component/module. Gather lines that relate to shared base/dify-ui component reuse, class names, styling/CSS imports, file size and component boundaries, i18n keys, behavior-sensitive UI interactions, React Flow hooks, and prop memoization.
2. For each rule in the review point, note where the code deviates and capture a representative snippet.
3. Compose the review section per the template below. Group violations first by **Urgent** flag, then by category order (Code Quality, Performance, Business Logic).

14
.agents/skills/frontend-code-review/references/code-quality.md
View File

@ -42,7 +42,17 @@ Category: Code Quality
When a frontend file grows large or mixes multiple responsibilities, suggest splitting it into focused components, hooks, or utilities. Prefer shallow local structure that matches existing repo patterns, such as a sibling `components/` folder, and avoid deep folder hierarchies unless the surrounding code already uses them.
Update this file when adding, editing, or removing Code Quality rules so the catalog remains accurate.
## Reuse base and dify-ui components before hand-rolling UI
Category: Code Quality
### Description
Before approving new or modified frontend UI, check whether the code manually recreates behavior or styling already owned by `@langgenius/dify-ui/*` or `web/app/components/base/*`. Common examples include `Button`, `Input`, `ToggleGroup`, `Popover`, `DropdownMenu`, `AlertDialog`, `Switch`, `Avatar`, `ScrollArea`, `toast`, and existing feature components. Prefer composing existing primitives instead of duplicating borders, focus states, disabled states, segmented controls, inputs, overlays, or buttons.
### Suggested Fix
Replace hand-written UI chrome with the nearest shared primitive, keeping feature-specific layout, state ownership, labels, and workflow behavior local.
## Classname ordering for easy overrides
@ -59,3 +69,5 @@ const Button = ({ className }) => {
return <div className={cn('bg-primary-600', className)}></div>
}
```
Update this file when adding, editing, or removing Code Quality rules so the catalog remains accurate.

79
docs/main-nav-gating-follow-ups.md
View File

@ -2,13 +2,21 @@
Context: the desktop MainNav rewrite moved several workspace, account, tools, and marketplace entry points out of the old header/account-setting layout. These notes track product-contract questions that should be resolved before treating the rewrite as behavior-complete.
Current status:
- Open: account-setting modal navigation API, Marketplace install/error status parity, default account language entry.
- Partially resolved: Apps/Datasets quick-switch/create parity.
- Resolved: Integrations sidebar placeholder state, branding-gated Help trigger, workspace plan billing access.
## 1. Account-setting modal naming and moved destinations
Status: Open.
Current branch behavior:
- `setShowAccountSettingModal(PROVIDER)` routes to `/tools?section=provider`.
- `setShowAccountSettingModal(DATA_SOURCE)` routes to `/tools?section=data-source`.
- `setShowAccountSettingModal(API_BASED_EXTENSION)` routes to `/tools?section=api-based-extension`.
- `setShowAccountSettingModal(PROVIDER)` routes to `/integrations/model-provider`.
- `setShowAccountSettingModal(DATA_SOURCE)` routes to `/integrations/data-source`.
- `setShowAccountSettingModal(API_BASED_EXTENSION)` routes to `/integrations/tools/api-extension`.
Old behavior: these calls opened the account-setting modal and switched to the matching tab.
@ -23,22 +31,26 @@ Follow-up decision needed:
## 2. Integrations sidebar disabled entries
Current branch behavior:
Status: Resolved.
Previous branch behavior:
- Integrations includes disabled entries for Trigger, Agent Strategy, and Extension.
- These are visible but not actionable.
Status:
Current branch behavior:
- Currently being handled separately.
- Trigger, Agent Strategy, and Extension are no longer disabled placeholders.
- These sections route to `PluginCategoryPage` with the corresponding plugin category.
Follow-up decision needed:
Resolution:
- Decide whether disabled future entries should remain visible, be hidden until supported, or be gated by feature flags/edition/role.
- If they stay visible, define the tooltip or disabled-state copy so users understand why the option is unavailable.
- No remaining disabled-entry gating decision is tracked here.
## 3. Plugin and marketplace status parity
Status: Open.
Old header behavior:
- `PluginsNav` showed plugin install progress and error state through the installing icon and red indicator.
@ -56,24 +68,29 @@ Follow-up decision needed:
## 4. Mobile/default account language entry
Status: Open.
Current branch behavior:
- Desktop MainNav account menu includes Language and Timezone submenus.
- Mobile still uses the default header/account dropdown path.
- The main app layout now uses MainNav across breakpoints.
- The default account dropdown does not expose the Language settings entry.
- The default account dropdown still exists in non-MainNav account/header surfaces such as the account layout.
Decision:
- Preserve the old language-access contract across breakpoints.
- The desktop MainNav path is acceptable; the missing case is mainly mobile/default account dropdown, including dataset-operator users on mobile.
- The desktop MainNav path is acceptable; the missing case is default account-dropdown parity wherever that path remains active.
Follow-up decision needed:
- Add an equivalent language entry to the default/mobile account path, or otherwise ensure mobile users can still reach language settings.
- Add an equivalent language entry to the default account path, or otherwise ensure users in those surfaces can still reach language settings.
- Keep this as gate-contract parity, not a visual requirement to recreate the old Account Settings sidebar.
## 5. Apps and Datasets quick-switch/create parity
Status: Partially resolved.
Old header behavior:
- `AppNav` could show the current app, list more apps, load more results, and launch create-app flows from the header nav.
@ -81,8 +98,9 @@ Old header behavior:
Current MainNav behavior:
- Apps and Datasets are static navigation links.
- App/dataset quick switching and create actions are not present in the desktop MainNav.
- Apps is no longer only a static navigation link: MainNav includes a Web Apps section with installed web app search, pin, delete, and navigation behavior.
- Apps still does not preserve the old `AppNav` current-app switcher, load-more behavior, or create-app flows.
- Datasets is still a navigation link and does not preserve the old `DatasetNav` current-dataset switcher, load-more behavior, or dataset creation entry.
Follow-up decision needed:
@ -91,6 +109,8 @@ Follow-up decision needed:
## 6. Branding-gated Help and Support behavior
Status: Resolved.
Old account dropdown behavior:
- When `systemFeatures.branding.enabled` is `true`, the whole Dify help/community group is hidden.
@ -98,21 +118,21 @@ Old account dropdown behavior:
Current MainNav behavior:
- HelpMenu keeps the trigger visible, but its content is gated by `!systemFeatures.branding.enabled`.
- This can produce an empty Help popup when branding is enabled.
- `HelpMenu` returns `null` when `systemFeatures.branding.enabled` is `true`.
- This prevents the empty Help trigger/popup path.
Open question:
Resolution:
- The old coarse gate also hides Support, but Support can contain instance-specific channels such as configured Zendesk or support email in addition to Dify forum/Discord links.
- Confirm whether branded deployments should hide Support entirely, or keep configured customer support channels while hiding Dify official/community links.
- Current implementation follows strict old parity for MainNav: the whole Help trigger is hidden for branded deployments.
Follow-up decision needed:
Optional future product question:
- If strict old parity is required, hide the HelpMenu trigger when branding is enabled.
- If branded deployments should retain support access, split Support into customer-support and Dify-community items with separate gates.
- If branded deployments should retain configured customer support channels, split Support into customer-support and Dify-community items with separate gates.
## 7. Paid plan Billing access from workspace plan
Status: Resolved.
Old header behavior:
- The header plan badge was clickable.
@ -122,17 +142,14 @@ Old header behavior:
Current MainNav behavior:
- Sandbox/free plans have an explicit Upgrade action in the WorkspaceCard credit row.
- Non-sandbox paid plans show the workspace plan badge, but the badge is display-only.
- Non-sandbox paid plans have an explicit View Plan action in the same plan-action row.
- Both actions open the pricing modal.
- The workspace plan badge is display-only.
- The WorkspaceCard Settings menu item routes to Account Settings on the Billing tab.
- Invite Members remains the Members entry, so Settings and Invite Members do not duplicate the same destination.
Open question:
Resolution:
- Confirm whether product still wants the plan badge itself to be clickable, or whether the Settings-to-Billing menu item is the intended MainNav access path.
- Avoid making the plan badge itself clickable unless the interaction is explicitly approved, because the WorkspaceCard already uses a button to open the workspace menu.
Recommended default:
- Keep sandbox/free behavior as the explicit Upgrade action.
- Keep the plan badge display-only.
- Keep sandbox/free and paid behavior as the explicit plan-action row.
- Keep the workspace plan badge display-only.
- Use the WorkspaceCard Settings item as the Billing entry.