-
Notifications
You must be signed in to change notification settings - Fork 314
Mobile apps
: Unify padding for problem statement components
#10986
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?
Mobile apps
: Unify padding for problem statement components
#10986
Conversation
Mobile apps
: Unify padding for problem statement componentsMobile apps
: Unify padding for problem statement components
End-to-End (E2E) Test Results Summary
|
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant ProblemStatementComponent
User->>Router: Navigates to problem statement route
Router->>ProblemStatementComponent: Initializes component
ProblemStatementComponent->>Router: Subscribes to route parameters and URL
Router-->>ProblemStatementComponent: Provides URL segments
ProblemStatementComponent->>ProblemStatementComponent: Sets isStandalone based on URL
ProblemStatementComponent->>ProblemStatementComponent: Renders container with conditional classes
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
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: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.html
(1 hunks)src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`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/**/*.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/core/course/overview/exercise-details/problem-statement/problem-statement.component.html
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts
🧬 Code Graph Analysis (1)
src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts (1)
src/test/javascript/spec/helpers/mocks/mock-router.ts (1)
url
(27-29)
🪛 GitHub Check: client-style
src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts
[warning] 48-48:
'url' is never reassigned. Use 'const' instead
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.html (1)
7-7
: LGTM! Conditional styling correctly implements mobile app padding.The ngClass binding properly applies padding (
p-3
) for standalone mobile contexts while maintaining the existing margin (mb-2
) for web contexts, which aligns perfectly with the PR objectives.src/main/webapp/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts (1)
2-2
: LGTM! NgClass import correctly added.The NgClass directive import and module registration are properly implemented to support the conditional styling in the template.
Also applies to: 17-17
...p/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts
Outdated
Show resolved
Hide resolved
...p/app/core/course/overview/exercise-details/problem-statement/problem-statement.component.ts
Outdated
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
End-to-End (E2E) Test Results Summary
|
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 TS3 and Ipad. Worked as described
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
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 TS4, looks good
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 TS2. Code LGTM. 😄
Checklist
General
Client
Motivation and Context
The mobile apps display problem statements for exercises using a separate problem statement component. This component however includes padding for programming exercises and no padding for other exercise types. This makes at least one of the types of exercises always look different from the other ones in the mobile apps.
Description
By adding padding to the non-programming exercise problem statements, we ensure that all types of problem statements appear with the same amount of padding (1rem).
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
Screenshots
Screenshots compare programming and text exercise – different (missing) padding can be observed. After this PR, all types should look like the programming exercise in terms of padding.
Summary by CodeRabbit