Addressed Context Center feedbacks#29230
Conversation
Rohit0301
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -264,6 +265,13 @@ const MemoryRow: FC<MemoryRowProps> = ({ | |||
| </Typography> | |||
| </Badge> | |||
| ))} | |||
| {memory?.tags?.length > VISIBLE_LINKED_ENTITIES_COUNT && ( | |||
| <Badge size="md" type="color"> | |||
| <Typography className="tw:text-secondary" size="text-xs"> | |||
| +{memory.tags.length - VISIBLE_LINKED_ENTITIES_COUNT} | |||
| </Typography> | |||
| </Badge> | |||
| )} | |||
There was a problem hiding this comment.
.slice() — all tags rendered alongside the overflow badge
VISIBLE_LINKED_ENTITIES_COUNT (4) is only used to compute the badge count; the .map() still iterates over every tag. When a memory has, say, 6 tags, the UI shows all 6 badges plus a "+2 more" badge simultaneously, which defeats the purpose of the constant. The fix is to add .slice(0, VISIBLE_LINKED_ENTITIES_COUNT) on the memory.tags array before calling .map(), matching the pattern already used in KnowledgeCard.tsx ((knowledgePage.tags ?? []).slice(0, 2).map(...)).
| useEffect(() => { | ||
| setTitleValue(value); | ||
| }, [value]); |
There was a problem hiding this comment.
useEffect can overwrite active user input when a debounced save resolves
The comment states this is safe because the parent only updates value after typing has settled, but the parent actually updates value when the API response resolves — not when typing stops. Concrete race condition: user types "Hello!", the debounce fires, a fast network returns displayName: "Hello!", then knowledgePage.displayName updates, and value prop changes. If the user resumed typing before the response arrived (e.g., "Hello! More text"), the useEffect fires setTitleValue("Hello!"), visibly resetting their cursor and truncating their input. The component should guard against overwriting unsaved local edits, for example by only syncing when titleValue already matches the previous prop value (i.e., when the user has not diverged from it).
🟡 Playwright Results — all passed (11 flaky)✅ 4007 passed · ❌ 0 failed · 🟡 11 flaky · ⏭️ 84 skipped
🟡 11 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Jest test CoverageUI tests summary
|
| <div className="custom-group tw:flex tw:flex-1 tw:items-center tw:gap-2 tw:min-w-0"> | ||
| <button | ||
| className="tw:flex tw:flex-1 tw:items-center tw:gap-2 tw:min-w-0 tw:text-left tw:bg-transparent tw:border-none tw:cursor-pointer tw:p-0" | ||
| className="tw:flex tw:flex-1 tw:items-center tw:gap-2 tw:min-w-0 tw:text-left tw:bg-transparent tw:border-none tw:cursor-pointer tw:p-0 tw:font-normal" |
There was a problem hiding this comment.
Reason for using a HTML button here?
| queryFilter: { | ||
| query: { term: { pageType: PageType.ARTICLE } }, | ||
| }, |
There was a problem hiding this comment.
We we are removing this? because we are already on KNOWLEDGE_PAGE_INDEX?
| {(tab) => ( | ||
| <Tabs.Item | ||
| {...tab} | ||
| className={({ isSelected }) => |
There was a problem hiding this comment.
we added isSelected prop on tab item no?
| export function decodeHtmlEntities(text: string): string { | ||
| const doc = new DOMParser().parseFromString(text, 'text/html'); | ||
|
|
||
| return doc.documentElement.textContent ?? text; | ||
| } |
There was a problem hiding this comment.
Did we try stress tests? like for very huge HTML how much time it will take?
Code Review ✅ Approved 4 resolved / 4 findingsRefines Context Center state management with concurrent save tracking and standardizes UI components like Breadcrumbs and ArchiveView. This update resolves previous issues regarding incorrect total counts and tag rendering. ✅ 4 resolved✅ Quality: Tag key changed from tagFQN to non-unique tag.name
✅ Bug: Total count counts all pages, mislabeled as articles
✅ Bug: Memory tags render all items plus a redundant overflow badge
✅ Quality: Truncated version title loses hover tooltip
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
2 participants
Describe your changes:
Screen.Recording.2026-06-22.at.1.05.56.PM.mov
Fixes 29275
I worked on ... because ...
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Summary by Gitar
ArchiveViewandMemoriesViewto use updated button components and improved icon layouts.KnowledgeCardandArticleDetailHeaderto standardize typography weights and iconography.maxItemWidthtoBreadcrumbsto prevent layout overflow in headers.pendingSaveCountRefinKnowledgePageDetailComponentto prevent premature status transitions toSAVEDduring concurrent debounced patches.TitleComponentwith external prop changes to ensure the input reflects the persisted display name.soft-delete-message-for-entityinen-us.jsonto reflect that soft-deleted items can be restored from Archives.ContextCentermodules to use consistent entity type labeling.This will update automatically on new commits.
Greptile Summary
This PR addresses a round of Context Center UI feedback: it stabilizes the save-state indicator during concurrent debounced saves, fixes article-count display in the hierarchy sidebar, and standardizes button/icon/typography tokens across ArchiveView, MemoriesView, and related components.
pendingSaveCountRefinKnowledgePageDetailComponentprevents the status badge from flipping toSAVEDwhile another debounced save is still in flight;setKnowledgePagecalls are switched to functional updaters to avoid stale-closure overwrites.KnowledgePagesHierarchynow fetches the total separately (limit=0, pageType=ARTICLE) instead of readingpaginationState.paging.total, so the sidebar count is always article-specific; Quick Links are hard-deleted while Articles are soft-deleted to Archives.soft-delete-archive-messagekey is added for Context Center soft-deletes, leaving the globalsoft-delete-message-for-entitykey untouched.Confidence Score: 5/5
Safe to merge; changes are UI/UX improvements and targeted logic fixes with no backend impact.
The core logic changes (pending-save counter, functional state updaters, separate article count fetch) are well-scoped and backed by tests. The one open question — whether removing the
PageType.ARTICLEfilter fromKnowledgePageListComponentis intentional — is a behavioral change worth confirming, but does not break anything on its own.KnowledgePageListComponent.tsx — the PageType.ARTICLE filter was removed from both the search and list API calls; worth confirming this is deliberate before merging.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant TitleComponent participant KnowledgePageDetailComponent participant API User->>TitleComponent: types display name TitleComponent->>KnowledgePageDetailComponent: handleDisplayNameOnChange(value) KnowledgePageDetailComponent->>KnowledgePageDetailComponent: pendingSaveCountRef++ (beginTrackedSave) KnowledgePageDetailComponent->>KnowledgePageDetailComponent: setContentChangeState(SAVING) User->>TitleComponent: types content (concurrent) TitleComponent->>KnowledgePageDetailComponent: handleContentOnChange(content) KnowledgePageDetailComponent->>KnowledgePageDetailComponent: pendingSaveCountRef++ (beginTrackedSave) KnowledgePageDetailComponent->>API: patchKnowledgePage (displayName) KnowledgePageDetailComponent->>API: patchKnowledgePage (content) API-->>KnowledgePageDetailComponent: response1 KnowledgePageDetailComponent->>KnowledgePageDetailComponent: "pendingSaveCountRef-- (endTrackedSave: count=1, no SAVED)" API-->>KnowledgePageDetailComponent: response2 KnowledgePageDetailComponent->>KnowledgePageDetailComponent: "pendingSaveCountRef-- (endTrackedSave: count=0, SAVED)" KnowledgePageDetailComponent->>TitleComponent: value prop updated TitleComponent->>TitleComponent: useEffect syncs titleValue from prop%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant TitleComponent participant KnowledgePageDetailComponent participant API User->>TitleComponent: types display name TitleComponent->>KnowledgePageDetailComponent: handleDisplayNameOnChange(value) KnowledgePageDetailComponent->>KnowledgePageDetailComponent: pendingSaveCountRef++ (beginTrackedSave) KnowledgePageDetailComponent->>KnowledgePageDetailComponent: setContentChangeState(SAVING) User->>TitleComponent: types content (concurrent) TitleComponent->>KnowledgePageDetailComponent: handleContentOnChange(content) KnowledgePageDetailComponent->>KnowledgePageDetailComponent: pendingSaveCountRef++ (beginTrackedSave) KnowledgePageDetailComponent->>API: patchKnowledgePage (displayName) KnowledgePageDetailComponent->>API: patchKnowledgePage (content) API-->>KnowledgePageDetailComponent: response1 KnowledgePageDetailComponent->>KnowledgePageDetailComponent: "pendingSaveCountRef-- (endTrackedSave: count=1, no SAVED)" API-->>KnowledgePageDetailComponent: response2 KnowledgePageDetailComponent->>KnowledgePageDetailComponent: "pendingSaveCountRef-- (endTrackedSave: count=0, SAVED)" KnowledgePageDetailComponent->>TitleComponent: value prop updated TitleComponent->>TitleComponent: useEffect syncs titleValue from propReviews (11): Last reviewed commit: "addressed gitar comment" | Re-trigger Greptile