-
Notifications
You must be signed in to change notification settings - Fork 358
Integrated markdown editor: Fix upload footer layout overflow
#12024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Integrated markdown editor: Fix upload footer layout overflow
#12024
Conversation
Integrated markdown editor: Integrated markdown editor: Fix upload footer layout overflow
|
@HawKhiem Test coverage has been automatically updated in the PR description. |
Integrated markdown editor: Integrated markdown editor: Fix upload footer layout overflowIntegrated markdown editor: Fix upload footer layout overflow
|
@HawKhiem Test coverage has been automatically updated in the PR description. |
WalkthroughRefactors the markdown editor to use a vertical flex layout and unifies editor height calculation to derive from the wrapper element, with clamped non-negative heights and string-based height binding (px or auto). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts`:
- Around line 517-522: getEditorHeight currently subtracts footer and palette
heights (using this.getElementClientHeight on this.wrapper,
this.fileUploadFooter, this.actionPalette) and then subtracts
BORDER_HEIGHT_OFFSET which can produce negative values in small containers;
update getEditorHeight to clamp the computed height to a minimum of 0 (e.g. use
Math.max(0, computedHeight)) before returning so Monaco never receives a
negative size.
src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts
Outdated
Show resolved
Hide resolved
|
@HawKhiem Test coverage has been automatically updated in the PR description. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
florian-glombik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
|
@HawKhiem Test coverage has been automatically updated in the PR description. |
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||
sawys777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed code and tested on TS4
anian03
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS6. The file upload footer was visible again in the exercise problem statement editor, and I could also not see any sizing issues anywhere else, e.g. communication or exam texts. When resizing, the text adapted to the available space and did not cause overflow issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS6, everything works as intended. After resizing, the text is still visible. Tested on Safari and Chrome.
|
@HawKhiem The "ready to merge" label was removed because the PR is not yet ready: Unchecked Tasks (1)
To resolve:
Once resolved, you can re-add the "ready to merge" label. |
|
@HawKhiem Your PR description needs attention before it can be reviewed: Issues Found
How to Fix
This check validates that your PR description follows the PR template. A complete description helps reviewers understand your changes and speeds up the review process.
|
maxgutke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this PR on TS1. Everything works as expected
|
@HawKhiem Test coverage has been automatically updated in the PR description. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
lana-ati
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested during working session on TS1. Worked as expected.
Checklist
General
Client
Changes affecting Programming Exercises
Motivation and Context
The "Info Text" at the top of the markdown editor was adding extra height that wasn't being accounted for when the editor was in
externalHeightmode (e.g., in the Instructor Code Editor). This caused the editor wrapper to inherit the full height of the container plus the info text, pushing the file upload footer out of the visible viewport.Description
This PR fixes the layout overflow issue by converting the markdown editor container to a Flexbox column.
d-flex flex-column. The Info Text is set toflex-shrink-0, and thewrapperis set toflex-grow-1. This ensures the wrapper automatically fills only the remaining space.getEditorHeight()to measurethis.wrapperinstead ofthis.fullElement. Since the wrapper is now correctly sized by Flexbox, this measurement inherently mimics the "available space" correctly, ensuring the internal Monaco instance sizes itself well within the boundaries.Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Client
Last updated: 2026-01-27 20:54:29 UTC
Screenshots
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.