Programming exercises: Fix table overflow in rendered markdown
#9957
Conversation
github-actions
bot
added
the
client
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/overview/course-overview.scss (1)
193-197: Consider extracting the magic number into a CSS variable.The width calculation uses a magic number (263px) which should be defined as a CSS variable for better maintainability.
+:root { + --sidebar-total-width: 263px; /* 255px sidebar + 8px margin */ +} .exercise-content-sidebar-width { - width: calc(100% - 263px) !important; + width: calc(100% - var(--sidebar-total-width)) !important; transition: width 0.1s 0s ease-in-out; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts(3 hunks)src/main/webapp/app/overview/course-exercises/course-exercises.component.html(2 hunks)src/main/webapp/app/overview/course-overview.scss(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/app/overview/course-exercises/course-exercises.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts (1)
🔇 Additional comments (3)
src/main/webapp/app/overview/course-exercises/course-exercises.component.html (1)
17-21: LGTM! Responsive layout implementation looks good.
The class bindings effectively handle the responsive layout:
- Base class
exercise-content-widthensures full width by default - Dynamic
exercise-content-sidebar-widthadjusts width when sidebar is visible
src/main/webapp/app/overview/course-overview.scss (1)
198-201: LGTM! Consistent transition implementation.
The transition timing matches between both classes, ensuring smooth width changes.
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts (1)
307-308: LGTM! Table styling integration looks good.
The integration of table styling in the markdown rendering process is implemented correctly.
Also applies to: 321-322
...ercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts (2)
333-335: Improve table detection condition readability.The condition could be more explicit to better convey its intent.
- if (!markdownWithoutTasks?.includes('<table')) { + if (!markdownWithoutTasks || !markdownWithoutTasks.includes('<table>')) { return; - } else { + }
332-347: Consider memory management in DOM manipulation.The current implementation could lead to memory leaks. Consider cleaning up DOM references and using a more functional approach.
- Clear any references to DOM elements after use
- Consider using a more functional approach that doesn't mutate state
- Document the memory management considerations in the method's JSDoc
Example JSDoc:
/** * Applies responsive styles to tables in markdown content. * @param markdownWithoutTasks - The markdown content to process * @returns The processed markdown content or undefined if no tables found * @note This method creates temporary DOM elements which are automatically cleaned up */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts (1)
🔇 Additional comments (2)
src/main/webapp/app/exercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts (2)
307-308: LGTM! Table styling is correctly integrated into the markdown rendering pipeline.
The changes properly handle table styling before sanitization while maintaining security through sanitizer.bypassSecurityTrustHtml.
Also applies to: 321-322
340-344: 🛠️ Refactor suggestion
Replace inline styles with CSS classes for better maintainability.
Using CSS classes instead of inline styles would improve maintainability and follow Angular best practices.
+// Add to your component's scss file
+.markdown-table {
+ max-width: 100%;
+ overflow-x: scroll;
+ display: block;
+}
- tables.forEach((table) => {
- table.style.maxWidth = '100%';
- table.style.overflowX = 'scroll';
- table.style.display = 'block';
- });
+ tables.forEach((table) => {
+ table.classList.add('markdown-table');
+ });Likely invalid or redundant comment.
...ercises/programming/shared/instructions-render/programming-exercise-instruction.component.ts
Show resolved
Hide resolved
rabeatwork
changed the title
Programming Exercises: Fix table overflow in rendered markdownProgramming exercises: Fix table overflow in rendered markdown
HawKhiem
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
az108
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
sawys777
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
ahbitaqu
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
SimonKaran13
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
bassner
deleted the
chore/programming-exercises/add-styles-for-tables-in-problem-statements
branch
Checklist
General
Client
Motivation and Context
When adding a large table to the problem statement the content of the exercise overflowed.
Description
Added logic to check if the rendered Markdown contains
<table>elements and applied styles dynamically to prevent overflow issues. Ensured tables are responsive by setting max-width: 100% and enabling horizontal scrolling with overflow-x: scroll.Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Screenshots
Before:
After:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style