✨ feat(editor): add sticky header bar filename on editor page#4573
Open
piedicianni wants to merge 8 commits into
Open
✨ feat(editor): add sticky header bar filename on editor page#4573piedicianni wants to merge 8 commits into
piedicianni wants to merge 8 commits into
Conversation
… and useSegmentsLoader
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 5 ❌ fail
Test File Matching: ✅ PASSFile matching: 5 pass 📋 5 files: 5 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 5 via AI (4 batches), 0 via shortcuts. 📋 5 files: 5 ✅ pass
Result: ✅ PASS |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a sticky “ProjectBar” (showing the current file name and word count) on the editor page, keeping it visible at the top while scrolling through segments and animating when the visible file changes.
Changes:
- Add support for rendering a sticky header in
VirtualListand wire it fromSegmentsContainerusing the first visible row id. - Refactor
RowSegmentto expose a reusableProjectBarcomponent with sticky/animation behavior. - Add new unit tests covering
VirtualList,RowSegment/ProjectBar,SegmentsContainer, anduseSegmentsLoader, plus required styling updates.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| public/js/components/common/VirtualList/Rows/RowSegment.js | Extracts ProjectBar and adds sticky + file-change animation behavior. |
| public/js/components/common/VirtualList/Rows/RowSegment.test.js | Adds tests for RowSegment and ProjectBar. |
| public/js/components/common/VirtualList/VirtualList.js | Adds header rendering and first-visible-row reporting. |
| public/js/components/common/VirtualList/VirtualList.test.js | Adds unit tests for VirtualList. |
| public/js/components/segments/SegmentsContainer.js | Renders the sticky ProjectBar based on the first visible virtual row. |
| public/js/components/segments/SegmentsContainer.test.js | Adds unit tests for sticky header integration and store wiring. |
| public/js/components/segments/SegmentFooter.js | Adds IntersectionObserver-gated rendering and height caching for the footer. |
| public/js/components/segments/SegmentFooter.test.js | Adds IntersectionObserver mock needed for the footer tests. |
| public/js/hooks/useSegmentsLoader.js | Adjusts loading flag handling while fetching segments. |
| public/js/hooks/useSegmentsLoader.test.js | Adds hook-level tests for fetching, flags, and cleanup. |
| public/css/sass/style.scss | Tweaks projectbar word-counter layout and loader z-index/margins. |
| public/css/sass/components/SegmentsContainer.scss | Adds styles and keyframes for the sticky project bar + animations. |
piedicianni
force-pushed
the
editor-filename-sticky-position
branch
from
aa32a34 to
e5955c8
Compare
piedicianni
changed the title
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 5 ❌ fail
Test File Matching: ✅ PASSFile matching: 5 pass 📋 5 files: 5 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 5 via AI (4 batches), 0 via shortcuts. 📋 5 files: 5 ✅ pass
Result: ✅ PASS |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (3)
public/js/components/common/VirtualList/Rows/RowSegment.js:202
- This block runs as a side effect during render, and mutates refs based on
listRef?.scrollTop. Because React render is not invoked on every scroll event,previousScrollTopRef.currentonly updates when the component re-renders for unrelated reasons, soisScrollingReverseRefwill rarely reflect the actual scroll direction at the moment a file change animation begins. This logic should live inside the scroll event handler (effect) where it actually observes scroll changes, instead of in the render body.
const previousScrollTopRef = useRef(0)
const isScrollingReverseRef = useRef(false)
if (isSticky && listRef?.scrollTop !== previousScrollTopRef.current) {
isScrollingReverseRef.current =
listRef?.scrollTop < previousScrollTopRef.current
}
previousScrollTopRef.current = listRef?.scrollTop
public/js/components/common/VirtualList/Rows/RowSegment.js:171
- When
isStickyis false (e.g. when ProjectBar is rendered inline by RowSegment),previousFileIdRef.currentis updated on every render, meaning the inline ProjectBar will also try to trigger a file-change animation/cleanup if the same instance gets reused with a different file id. The whole effect (includingpreviousFileIdRef.current = idFileSegment) should be guarded byisSticky, otherwise the bookkeeping diverges from the intent (animation only when sticky).
useEffect(() => {
let tmOutReset
const stopFileChangeAnim = () => setIsFileChange(false)
const filenameElement = ref.current.firstChild
const wordcounterElement = ref.current.children[1]
if (
isSticky &&
idFileSegment !== previousFileIdRef.current &&
typeof previousFileIdRef.current === 'number'
) {
ref.current.style.animation = 'none'
if (filenameElement) filenameElement.style.animation = 'none'
if (wordcounterElement) wordcounterElement.style.animation = 'none'
tmOutReset = setTimeout(() => {
ref.current.style.animation = ''
if (filenameElement) filenameElement.style.animation = ''
if (wordcounterElement) wordcounterElement.style.animation = ''
}, 0)
filenameElement?.addEventListener('animationend', stopFileChangeAnim)
setIsFileChange(true)
}
previousFileIdRef.current = idFileSegment
return () => {
clearTimeout(tmOutReset)
filenameElement?.removeEventListener('animationend', stopFileChangeAnim)
stopFileChangeAnim()
}
}, [isSticky, idFileSegment])
public/js/components/common/VirtualList/Rows/RowSegment.js:158
ref.current.firstChildandref.current.children[1]are read unconditionally at the top of the effect, but the rendered children are conditional onfileandfile.weighted_words > 0. Whenfileis falsy,ref.current.firstChildwill be the next conditional element (or null), andchildren[1]may not be the wordcounter at all, leading to misappliedstyle.animation = 'none'resets on the wrong nodes. Compute these references inside theif (isSticky && ...)branch and after verifying the elements exist by class.
const filenameElement = ref.current.firstChild
const wordcounterElement = ref.current.children[1]
if (
isSticky &&
idFileSegment !== previousFileIdRef.current &&
typeof previousFileIdRef.current === 'number'
) {
ref.current.style.animation = 'none'
if (filenameElement) filenameElement.style.animation = 'none'
if (wordcounterElement) wordcounterElement.style.animation = 'none'
tmOutReset = setTimeout(() => {
ref.current.style.animation = ''
if (filenameElement) filenameElement.style.animation = ''
if (wordcounterElement) wordcounterElement.style.animation = ''
}, 0)
| const file = files ? files.find((file) => file.id == idFileSegment) : false | ||
| let fileType = '' | ||
| if (file) { | ||
| fileType = file.file_name ? file.file_name.split('.')[1] : '' |
| } | ||
|
|
||
| setHeight(sid, value) { | ||
| this.map.clear() |
Comment on lines
+661
to
+663
| if (canDisplayContent && ref.current) { | ||
| segmentFooterHeight.setHeight(segment.sid, ref.current.offsetHeight) | ||
| } |
Comment on lines
79
to
+131
| @@ -76,6 +94,7 @@ const VirtualList = forwardRef( | |||
| position: 'relative', | |||
| }} | |||
| > | |||
| {overlapHeader && overlapHeader} | |||
| {virtualItems.map((item) => ( | |||
| <div | |||
| key={items[item.index].id ? items[item.index].id : item.index} | |||
| @@ -109,6 +128,8 @@ VirtualList.propTypes = { | |||
| align: PropTypes.string, | |||
| }), | |||
| onRender: PropTypes.func.isRequired, | |||
| setFirstRowIdVisible: PropTypes.func.isRequired, | |||
Comment on lines
34
to
+81
| @@ -62,6 +76,10 @@ const VirtualList = forwardRef( | |||
| }` | |||
| }, [ref, width, height]) | |||
|
|
|||
| useEffect(() => { | |||
| setFirstRowIdVisible(firstRowIdVisible) | |||
| }, [firstRowIdVisible, setFirstRowIdVisible]) | |||
| openSegmentComment: jest.fn(), | ||
| setBulkSelectionInterval: jest.fn(), | ||
| getMoreSegments: jest.fn(), | ||
| scrollToCurrentSegment: jest.fn(), |
Comment on lines
+846
to
+858
| if (typeof firstRowIdVisible !== 'undefined') { | ||
| const props = getSegmentPropsBySid(firstRowIdVisible) | ||
|
|
||
| return ( | ||
| <div | ||
| className={`sticky-project-bar ${props.sideOpen ? 'sticky-project-bar-slide-right' : ''}`} | ||
| > | ||
| <ProjectBar | ||
| {...{...props, listRef: listRef.current, isSticky: true}} | ||
| /> | ||
| </div> | ||
| ) | ||
| } |
| ...(index === essentialRows.length - 1 && { | ||
| isLastRow: true, | ||
| }), | ||
| scrollValue: listRef.current ? listRef.current.scrollTop : 0, |
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 4 files: 4 ❌ fail
Test File Matching: ✅ PASSFile matching: 4 pass 📋 4 files: 4 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 4 files: 4 via AI (4 batches), 0 via shortcuts. 📋 4 files: 4 ✅ pass
Result: ✅ PASS |
🧪 Test-Guard ReportCoverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 4 files: 4 ❌ fail
Test File Matching: ✅ PASSFile matching: 4 pass 📋 4 files: 4 ✅ pass
Per-File Evaluation:
|
| File | Verdict | Reason |
|---|---|---|
public/js/components/common/VirtualList/Rows/RowSegment.js |
✅ pass | Removed ProjectBar function; tests cover its rendering and scroll behavior thoroughly. |
public/js/components/common/VirtualList/VirtualList.js |
✅ pass | Tests verify rendering, scroll behavior, header rendering, and setFirstRowIdVisible callback. |
public/js/components/segments/SegmentFooter.js |
Tests mock IntersectionObserver and render component, but no direct height logic tests. | |
public/js/components/segments/SegmentsContainer.js |
✅ pass | New ProjectBar sticky rendering and row height margin logic tested indirectly via matched tests. |
Result:
Why this WARNING?
- Coverage Analysis: Four changed source files (RowSegment.js, VirtualList.js, SegmentFooter.js, SegmentsContainer.js) are missing from the coverage report, indicating a coverage instrumentation or configuration issue → update coverage configuration to include these files.
- Per-File Evaluation: SegmentFooter.js tests mock IntersectionObserver and render the component but do not directly test height-related logic → add targeted tests for height logic in SegmentFooter.js.
- Test File Matching: All changed source files have matching test files modified in the PR → no action needed.
- Per-File Evaluation: Other files (RowSegment.js, VirtualList.js, SegmentsContainer.js) have adequate test coverage and logic testing → no action needed.
To resolve: fix coverage instrumentation/configuration to include the changed source files and add direct tests for height logic in SegmentFooter.js.
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
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.
Summary
Add sticky project bar with filename on the editor page. The filename header bar remains visible and fixed at the top while scrolling through segments, with smooth animations when the file changes.
Type
feat— new user-facing featurefix— bug fixrefactor— restructure without behavior changechore— build, deps, config, docsperf— performance improvementtest— test coverageChanges
Testing
vendor/bin/phpunit --exclude-group=ExternalServices --no-coveragepasses./vendor/bin/phpstanpasses (0 errors, with baseline)Manual testing:
AI Disclosure
GitHub Copilot (claude-sonnet-4-6)
Notes
The sticky bar shows the current file name and word count while scrolling. An animation is triggered when the visible file changes.