[WIKI-608] Change the header of the pages to be more generic #7787
Uh oh!
< FF8 p data-view-component="true">There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughIntroduces a new BasePagesListHeaderRoot component handling search, sort, and filters via shared page store. Refactors PagesListHeaderRoot into a thin wrapper delegating to the base component and removing the storeType prop. Updates PagesListView to use the new header signature without storeType. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant V as PagesListView
participant H as PagesListHeaderRoot
participant B as BasePagesListHeaderRoot
participant S as Page Store (PROJECT)
participant M as useMember
U->>V: Load Pages list view
V->>H: Render header (pageType, projectId, workspaceSlug)
H->>B: Render base header (storeType=PROJECT, tabNavigationComponent)
B->>S: Read filters, order, search
B->>M: Fetch workspace member IDs
Note over B,S: Primary header renders search, sort, filters
U->>B: Search / Sort / Open Filters
B->>S: updateFilters(...) / change order / search query
alt Any filters applied
B->>S: calculateTotalFilters()
Note over B,S: Secondary applied-filters bar shown
U->>B: Remove a chip / Clear all
opt Remove specific filter
B->>S: updateFilters(remove key/value) [favorites handled specially]
end
opt Clear all
B->>S: clearAllFilters()
end
else No filters
Note over B: Secondary bar hidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/core/components/pages/header/base.tsx (1)
50-50: Reuse computed flag instead of recomputing filter countUse
isFiltersAppliedin both places to avoid duplicate work and ensure consistent logic.- const isFiltersApplied = calculateTotalFilters(filters?.filters ?? {}) !== 0; + const isFiltersApplied = calculateTotalFilters(filters?.filters ?? {}) !== 0; ... - {calculateTotalFilters(filters?.filters ?? {}) !== 0 && ( + {isFiltersApplied && (Also applies to: 83-93
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/core/components/pages/header/base.tsx(1 hunks)apps/web/core/components/pages/header/root.tsx(1 hunks)apps/web/core/components/pages/pages-list-view.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/core/components/pages/pages-list-view.tsx (1)
apps/web/core/components/pages/header/root.tsx (1)
PagesListHeaderRoot(16-27)
apps/web/core/components/pages/header/base.tsx (5)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)packages/types/src/page/core.ts (1)
TPageFilterProps(34-39)apps/web/core/components/issues/issue-layouts/filters/header/helpers/dropdown.tsx (1)
FiltersDropdown(25-124)apps/web/core/store/member/workspace-member.store.ts (1)
workspaceMemberIds(97-102)
apps/web/core/components/pages/header/root.tsx (2)
packages/types/src/page/core.ts (1)
TPageNavigationTabs(28-28)apps/web/core/components/pages/header/base.tsx (1)
BasePagesListHeaderRoot(26-95)
🔇 Additional comments (2)
apps/web/core/components/pages/header/base.tsx (1)
52-82: Nice separation of concerns; base header composition looks solidClear split between navigation (left) and controls (right), and all mutations flow through the store. Good use of observer and small subcomponents.
apps/web/core/components/pages/header/root.tsx (1)
16-26: Wrapper refactor is clean and minimalThin wrapper delegating to Base keeps responsibilities clear and reduces duplication.
Comment on lines
+35
to
+48
| const handleRemoveFilter = useCallback( | ||
| (key: keyof TPageFilterProps, value: string | null) => { | ||
| let newValues = filters.filters?.[key]; | ||
|
|
||
| if (key === "favorites") newValues = !!value; | ||
| if (Array.isArray(newValues)) { | ||
| if (!value) newValues = []; | ||
| else newValues = newValues.filter((val) => val !== value); | ||
| } | ||
|
|
||
| updateFilters("filters", { [key]: newValues }); | ||
| }, | ||
| [filters.filters, updateFilters] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix favorites removal and improve type safety in handleRemoveFilter
Setting favorites to false on removal may leave a redundant key and relies on downstream logic to ignore it. Prefer clearing the key (undefined) and narrow types per-key to avoid boolean/array confusion.
- const handleRemoveFilter = useCallback(
- (key: keyof TPageFilterProps, value: string | null) => {
- let newValues = filters.filters?.[key];
-
- if (key === "favorites") newValues = !!value;
- if (Array.isArray(newValues)) {
- if (!value) newValues = [];
- else newValues = newValues.filter((val) => val !== value);
- }
-
- updateFilters("filters", { [key]: newValues });
- },
- [filters.filters, updateFilters]
- );
+ const handleRemoveFilter = useCallback(
+ (key: keyof TPageFilterProps, value: string | null) => {
+ const current = filters.filters ?? {};
+ // Special-case boolean filters
+ if (key === "favorites") {
+ // remove when value is null, otherwise set true
+ updateFilters("filters", { favorites: value === null ? undefined : true });
+ return;
+ }
+ const existing = current[key as Exclude<keyof TPageFilterProps, "favorites">];
+ let next: unknown = existing;
+ if (Array.isArray(existing)) {
+ next = value ? existing.filter((v) => v !== value) : [];
+ } else {
+ // Non-array keys: clear by default
+ next = undefined;
+ }
+ updateFilters("filters", { [key]: next } as Partial<TPageFilterProps>);
+ },
+ [filters.filters, updateFilters]
+ );📝 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.
Suggested change
| const handleRemoveFilter = useCallback( | |
| (key: keyof TPageFilterProps, value: string | null) => { | |
| let newValues = filters.filters?.[key]; | |
| if (key === "favorites") newValues = !!value; | |
| if (Array.isArray(newValues)) { | |
| if (!value) newValues = []; | |
| else newValues = newValues.filter((val) => val !== value); | |
| } | |
| updateFilters("filters", { [key]: newValues }); | |
| }, | |
| [filters.filters, updateFilters] | |
| ); | |
| const handleRemoveFilter = useCallback( | |
| (key: keyof TPageFilterProps, value: string | null) => { | |
| const current = filters.filters ?? {}; | |
| // Special-case boolean filters | |
| if (key === "favorites") { | |
| // remove when value is null, otherwise set true | |
| updateFilters("filters", { favorites: value === null ? undefined : true }); | |
| return; | |
| } | |
| const existing = current[key as Exclude<keyof TPageFilterProps, "favorites">]; | |
| let next: unknown = existing; | |
| if (Array.isArray(existing)) { | |
| next = value ? existing.filter((v) => v !== value) : []; | |
| } else { | |
| // Non-array keys: clear by default | |
| next = undefined; | |
| } | |
| updateFilters("filters", { [key]: next } as Partial<TPageFilterProps>); | |
| }, | |
| [filters.filters, updateFilters] | |
| ); |
🤖 Prompt for AI Agents
In apps/web/core/components/pages/header/base.tsx around lines 35–48,
handleRemoveFilter currently sets the "favorites" filter to a boolean (false)
and uses a union-typed newValues which can leave a redundant key and cause
downstream confusion; change the removal behavior for "favorites" to clear the
key (undefined) instead of setting false, and narrow the per-key types before
manipulating values (e.g., if key === "favorites" treat current value as boolean
| undefined and set newValues = undefined on removal; otherwise treat as
string[] and filter or set to []), then call updateFilters with the
properly-typed partial update so the favorites key is removed rather than left
false.
Comment on lines
+21
to
+25
| storeType={EPageStoreType.PROJECT} | ||
| tabNavigationComponent={ | ||
| <PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} /> | ||
| } | ||
| /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t hard-code PROJECT; thread storeType through to Base
Hard-coding ties the “generic” header to project scope and can desync from views using other stores. Accept an optional storeType (default to PROJECT) and pass it through.
type Props = {
pageType: TPageNavigationTabs;
projectId: string;
workspaceSlug: string;
+ storeType?: EPageStoreType;
};
export const PagesListHeaderRoot: React.FC<Props> = observer((props) => {
- const { pageType, projectId, workspaceSlug } = props;
+ const { pageType, projectId, workspaceSlug, storeType = EPageStoreType.PROJECT } = props;
return (
<BasePagesListHeaderRoot
- storeType={EPageStoreType.PROJECT}
+ storeType={storeType}
tabNavigationComponent={
<PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} />
}
/>
);
});After this change, pass storeType from PagesListView (see companion comment).
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo root ==="
pwd
echo
echo "=== show file if exists: apps/web/core/components/pages/header/root.tsx ==="
if [ -f "apps/web/core/components/pages/header/root.tsx" ]; then
sed -n '1,240p' apps/web/core/components/pages/header/root.tsx
else
echo "MISSING: apps/web/core/components/pages/header/root.tsx"
fi
echo
echo "=== search for related symbols ==="
rg -n --hidden -S 'PagesListHeaderRoot' -g '!*node_modules/*' -C3 || true
rg -n --hidden -S 'BasePagesListHeaderRoot' -g '!*node_modules/*' -C3 || true
rg -n --hidden -S 'EPageStoreType' -g '!*node_modules/*' -C3 || true
rg -n --hidden -S 'PagesListView' -g '!*node_modules/*' -C3 || true
rg -n --hidden -S 'PageTabNavigation' -g '!*node_modules/*' -C3 || trueLength of output: 1674
Don't hard-code PROJECT — thread storeType through to Base
Hard-coding ties the generic header to project scope and can desync from views using other stores. Accept an optional storeType (default to EPageStoreType.PROJECT) and pass it to BasePagesListHeaderRoot.
File: apps/web/core/components/pages/header/root.tsx (around lines 21–25)
type Props = {
pageType: TPageNavigationTabs;
projectId: string;
workspaceSlug: string;
+ storeType?: EPageStoreType;
};
export const PagesListHeaderRoot: React.FC<Props> = observer((props) => {
- const { pageType, projectId, workspaceSlug } = props;
+ const { pageType, projectId, workspaceSlug, storeType = EPageStoreType.PROJECT } = props;
return (
<BasePagesListHeaderRoot
- storeType={EPageStoreType.PROJECT}
+ storeType={storeType}
tabNavigationComponent={
<PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} />
}
/>
);
});After this change, pass storeType from PagesListView (see companion comment).
📝 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.
Suggested change
| storeType={EPageStoreType.PROJECT} | |
| tabNavigationComponent={ | |
| <PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} /> | |
| } | |
| /> | |
| type Props = { | |
| pageType: TPageNavigationTabs; | |
| projectId: string; | |
| workspaceSlug: string; | |
| storeType?: EPageStoreType; | |
| }; | |
| export const PagesListHeaderRoot: React.FC<Props> = observer((props) => { | |
| const { pageType, projectId, workspaceSlug, storeType = EPageStoreType.PROJECT } = props; | |
| return ( | |
| <BasePagesListHeaderRoot | |
| storeType={storeType} | |
| tabNavigationComponent={ | |
| <PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} /> | |
| } | |
| /> | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In apps/web/core/components/pages/header/root.tsx around lines 21 to 25, the
component currently hard-codes storeType=EPageStoreType.PROJECT; change the
component signature to accept an optional prop storeType with a default of
EPageStoreType.PROJECT, and forward that prop to the BasePagesListHeaderRoot
(replace the hard-coded value with the new prop). After this change, update the
PagesListView to pass its storeType through into this header component so the
header reflects the actual view's store type.
| storeType={storeType} | ||
| workspaceSlug={workspaceSlug} | ||
| /> | ||
| <PagesListHeaderRoot pageType={pageType} projectId={projectId} workspaceSlug={workspaceSlug} /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Header store now diverges from view store (PROJECT vs storeType)
Header is hard-coded to the PROJECT store via the wrapper, while this view gates and fetches via storeType and renders content with storeType. This can desync filters/search/sort and even hide the header when storeType !== PROJECT. Pass storeType through to keep header and content in the same store.
Apply:
- <PagesListHeaderRoot pageType={pageType} projectId={projectId} workspaceSlug={workspaceSlug} />
+ <PagesListHeaderRoot
+ pageType={pageType}
+ projectId={projectId}
+ workspaceSlug={workspaceSlug}
+ storeType={storeType}
+ />To verify current usages and whether non-PROJECT store types exist:
🏁 Script executed:
#!/bin/bash
# Find PagesListView call sites and inspect storeType usage.
rg -nP --type tsx -C3 '<PagesListView\b' | sed -n '1,200p'
rg -nP --type tsx -C3 'storeType\s*=' | sed -n '1,200p'Length of output: 55
🏁 Script executed:
#!/bin/bash
set -o pipefail
echo "=== search for PagesListView / PagesListHeaderRoot usages ==="
rg -n --hidden -S '<PagesListView\b' || true
rg -n --hidden -S '<PagesListHeaderRoot\b' || true
echo
echo "=== search for filenames / symbols ==="
rg -n --hidden -S 'pages-list-view|pages-list-header-root|PagesListHeaderRoot|PagesListView' || true
echo
echo "=== show specific file if it exists ==="
if [ -f "apps/web/core/components/pages/pages-list-view.tsx" ]; then
echo "---- apps/web/core/components/pages/pages-list-view.tsx ----"
sed -n '1,240p' apps/web/core/components/pages/pages-list-view.tsx || true
else
echo "file apps/web/core/components/pages/pages-list-view.tsx not found"
fi
echo
echo "=== find files that define PagesListHeaderRoot and print head ==="
files=$(rg -l --hidden -S 'PagesListHeaderRoot' || true)
if [ -n "$files" ]; then
for f in $files; do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
echo
done
else
echo "No files named PagesListHeaderRoot found by rg"
fi
echo
echo "=== search for storeType / StoreType.PROJECT mentions ==="
rg -n --hidden -S 'storeType|StoreType\.PROJECT' || true
echo
echo "=== list files in apps/web/core/components/pages if present ==="
if [ -d "apps/web/core/components/pages" ]; then
ls -la apps/web/core/components/pages || true
else
echo "dir apps/web/core/components/pages not found"
fiLength of output: 40506
Pass storeType through to the header and make the header respect it
Header is hard-coded to EPageStoreType.PROJECT while PagesListView fetches/renders by the incoming storeType — this can desync filters/search/sort or hide the header when storeType !== PROJECT. Change both files so header receives and uses the same storeType.
- Apply to apps/web/core/components/pages/pages-list-view.tsx:
- <PagesListHeaderRoot pageType={pageType} projectId={projectId} workspaceSlug={workspaceSlug} />
+ <PagesListHeaderRoot
+ pageType={pageType}
+ projectId={projectId}
+ workspaceSlug={workspaceSlug}
+ storeType={storeType}
+ />- Update apps/web/core/components/pages/header/root.tsx to accept and forward storeType (keep PROJECT as default for backwards compatibility):
-type Props = {
- pageType: TPageNavigationTabs;
- projectId: string;
- workspaceSlug: string;
-};
+type Props = {
+ pageType: TPageNavigationTabs;
+ projectId: string;
+ workspaceSlug: string;
+ storeType?: EPageStoreType;
+};
export const PagesListHeaderRoot: React.FC<Props> = observer((props) => {
- const { pageType, projectId, workspaceSlug } = props;
+ const { pageType, projectId, workspaceSlug, storeType = EPageStoreType.PROJECT } = props;
return (
<BasePagesListHeaderRoot
- storeType={EPageStoreType.PROJECT}
+ storeType={storeType}
tabNavigationComponent={
<PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} />
}
/>
);
});📝 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.
Suggested change
| <PagesListHeaderRoot pageType={pageType} projectId={projectId} workspaceSlug={workspaceSlug} /> | |
| // apps/web/core/components/pages/pages-list-view.tsx | |
| <PagesListHeaderRoot | |
| pageType={pageType} | |
| projectId={projectId} | |
| workspaceSlug={workspaceSlug} | |
| storeType={storeType} | |
| /> |
Suggested change
| <PagesListHeaderRoot pageType={pageType} projectId={projectId} workspaceSlug={workspaceSlug} /> | |
| // apps/web/core/components/pages/header/root.tsx | |
| type Props = { | |
| pageType: TPageNavigationTabs; | |
| projectId: string; | |
| workspaceSlug: string; | |
| storeType?: EPageStoreType; | |
| }; | |
| export const PagesListHeaderRoot: React.FC<Props> = observer((props) => { | |
| const { pageType, projectId, workspaceSlug, storeType = EPageStoreType.PROJECT } = props; | |
| return ( | |
| <BasePagesListHeaderRoot | |
| storeType={storeType} | |
| tabNavigationComponent={ | |
| <PageTabNavigation workspaceSlug={workspaceSlug} projectId={projectId} pageType={pageType} /> | |
| } | |
| /> | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In apps/web/core/components/pages/pages-list-view.tsx around line 33 and in
apps/web/core/components/pages/header/root.tsx, the PagesListHeaderRoot is
hard-coded to EPageStoreType.PROJECT causing desync when pages-list-view uses a
different storeType; update pages-list-view to pass the current storeType prop
into PagesListHeaderRoot, and update header/root.tsx to accept an optional
storeType prop (defaulting to EPageStoreType.PROJECT for backwards
compatibility) and forward that storeType to any child/header internals that
currently assume PROJECT so filters/search/sort reflect the incoming storeType.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Change the header of the pages to be more generic
Type of Change
Summary by CodeRabbit
New Features
Refactor