[Bugfix:TAGrading] Fix pdf file view visibility#12779
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12779 +/- ##
============================================
- Coverage 21.73% 21.66% -0.07%
- Complexity 9660 9754 +94
============================================
Files 268 268
Lines 36273 36457 +184
Branches 487 487
============================================
+ Hits 7884 7900 +16
- Misses 27906 28074 +168
Partials 483 483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Hey @williamschen23 , I could reproduce the same error in my branch from PR #12766. When I tried again on your branch there was no error. So the PR removes the bug Also regarding the bottom scroll, I wanted to say that this scroll is not due to your file changes, it was there in my PR as well. I tried to find the cause of it. Bottom scroll is caused by PDF content overflow in #viewer at this narrow width. I think this is expected narrow-width PDF behaviour. An issue I observed (which is maybe out of scope of this PR) is that when in mobile view seeing the PDF is very difficult due to this PDF header overlap. No doubt by this PR and the #12766 we were solving the issue #12763 and now the PDF no longer remains blocked, one can see the PDF if he scrolls enough but it looks very messy with the PDF header at narrow widht. Screen.Recording.2026-04-13.155039.mp4I have attached a screen recording of that.
|
|
closed bc PR got reverted |
### Why is this Change Important & Necessary? Solves #12763 #12766 changes were reverted by the PR #12784 due to a bug identified in the PR #12779 In PR #12779 in mobile view seeing the PDF is very difficult due to this PDF header overlap. Screen recording of that- https://github.com/user-attachments/assets/f8c0b147-e8b9-494c-b763-ab772d9e2cd0 ### What is the New Behavior? This PR includes all the changes from #12766 while excluding the unnecessary portion identified in #12779. Also it fixes the poor PDF readability in mobile view. https://github.com/user-attachments/assets/78a5cce8-3a93-45c4-8f7a-57428873a8e2 ### What steps should a reviewer take to reproduce or test the bug or new feature? 1) Open TA Grading for a gradeable with PDF submissions. 2) Open a PDF in the Submissions and Results Browser. 3) Verify the top file header remains usable while scrolling the PDF content. 4) Resize browser to narrow/mobile width. 5) Confirm horizontal overflow is not locking the view and PDF remains scrollable/usable. 6) Use page navigation/jump behavior and confirm the expected page scroll target is reached. ### Automated Testing & Documentation No new automated tests were added in this PR (UI/layout and scroll behavior adjustment). ### Other information Breaking change: No. Migrations required: No. Security concerns: None identified. --------- Co-authored-by: Barb Cutler <[email protected]>



Why is this Change Important & Necessary?
Bug from #12766

Bug: pdf file view is always available, which messes up other code:
const activeView = $('#file-view').is(':visible'); let lastLoadedFile = activeView ? $('#grading_file_name').text().trim() : localStorage.getItem('ta-grading-files-full-view-last-opened') ?? 'upload.pdf';if file view is available, it will get the file name, which doesnt exist because file view has nothing
What is the New Behavior?
there was no need for display flex, which brought up the file view.
NOTE: this is still a little buggy, as the pdf scroll location is at the bottom somehow?
What steps should a reviewer take to reproduce or test the bug or new feature?
Make sure original functionality form #12766 works, Im pretty sure it does
Automated Testing & Documentation
Other information