Skip to content

fix: correct requestScrollTo scroll offset calculations#2927

Open
cqnykamp wants to merge 1 commit into
mainfrom
claude/modest-franklin-DXoke
Open

fix: correct requestScrollTo scroll offset calculations#2927
cqnykamp wants to merge 1 commit into
mainfrom
claude/modest-franklin-DXoke

Conversation

@cqnykamp

Copy link
Copy Markdown
Contributor

Summary

Fixes #2604requestScrollTo scroll offset calculations were incorrect in two places, causing DoenetML content to scroll to the wrong position when the viewer requests a scroll (e.g., when navigating to an answer or hint).

  • AssignmentResponseStudent.tsx: The FIXED_OFFSET was 0 (a placeholder - 0) instead of 90, causing content to scroll too far down. Additionally, the viewerContainer ref was attached to the outer <GridItem area="centerContent"> rather than the actual viewer Box, so getBoundingClientRect().top measured the wrong element and the scroll target was miscalculated.
  • AssignmentItemResponseStudent.tsx: Added an optional viewerContainerRef prop (forwarded to the viewer Box) so AssignmentResponseStudent can measure the correct element.
  • DocEditorViewMode.tsx: The offset was hardcoded as 90 (matching the normal 80px header + 10px margin), but when notBrowsable=true a 40px warning banner is added making headerHeight="120px". The hardcoded value caused the scroll target to land 30px above the visible area. Changed to parseInt(headerHeight) + 10 to use the dynamic value.

Test plan

  • Open an assignment as a student, navigate to an item with hints or answer feedback; verify the page scrolls to the correct position
  • Open an assignment response review page (AssignmentResponseStudent) and verify scroll-to works
  • Open a document in the editor view mode with a notBrowsable warning banner (set notBrowsable=true) and verify scroll-to works correctly with the taller header

Notes

Cypress component tests could not be run locally (binary unavailable in this environment). TypeScript (tsc --noEmit) and ESLint both pass. CI will run the full test suite.

The existing cy.get("iframe").eq(0).scrollIntoView() workaround in assignmentWorkflow.cy.ts (around lines 364–394) is a separate pre-existing issue related to @doenet/assignment-viewer (DoenetActivityViewer) behavior when creating new item attempts — not addressed by this PR.

https://claude.ai/code/session_01JMg997rhUoDWpN7mscG1eJ


Generated by Claude Code

- AssignmentResponseStudent: fix FIXED_OFFSET from 0 to 90 and point
  viewerContainer ref at the actual viewer Box instead of the outer
  GridItem, so the scroll target position is calculated correctly
- AssignmentItemResponseStudent: accept optional viewerContainerRef prop
  and attach it to the viewer Box so the parent can measure its position
- DocEditorViewMode: use dynamic headerHeight instead of hardcoded 90
  so scroll works correctly when the notBrowsable warning banner is shown
  (headerHeight becomes 120px, adding 40px to the fixed header)

Fixes #2604

https://claude.ai/code/session_01JMg997rhUoDWpN7mscG1eJ

Copy link
Copy Markdown
Contributor Author

The component-tests (group1) CI failure appears to be pre-existing and unrelated to this PR.

My changes only touch three files:

  • apps/app/src/paths/AssignmentResponseStudent.tsx
  • apps/app/src/views/AssignmentItemResponseStudent.tsx
  • apps/app/src/paths/editor/DocEditorViewMode.tsx

None of these files are imported or referenced by any @group1 component test. The group1 suite covers unrelated components: AnswerResponseDrawer, ContentInfoDrawer, CreateContentMenu, DoenetMLVersionComponents, GeneralContentInfo, ExploreFilterDrawer, ContributorsMenu, EditorLibraryMode, EditorSettingsMode, DocEditorRemixMode, Remixes, RemixSources, ChangeName, DocEditorHistoryMode, and ClassificationInfo.

All other CI checks (e2e groups 1–4, brittle groups 1–3, component groups 2–4, and the format/lint/build/unit check) either passed or are still running.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Scrolling bug

2 participants