Programming Exercises: Align title and buttons for participation page
#9983
Conversation
github-actions
bot
added
the
client
WalkthroughThe changes in this pull request focus on the Changes
Assessment against linked issues
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: 0
🧹 Outside diff range and nitpick comments (2)
src/main/webapp/app/exercises/shared/participation/participation.component.html (2)
32-51: Consider enhancing accessibility for radio button group.While the layout is good, consider wrapping the radio buttons in a
fieldsetwith alegendfor better semantic structure and accessibility.- <div class="d-flex align-items-center mt-2"> + <fieldset class="d-flex align-items-center mt-2 border-0 p-0"> + <legend class="visually-hidden">Filter participations</legend> <label class="radio-inline mb-0 d-flex align-items-center"> - <input type="radio" [ngModel]="participationCriteria.filterProp" (click)="updateParticipationFilter(FilterProp.ALL)" [value]="FilterProp.ALL" /> + <input type="radio" [ngModel]="participationCriteria.filterProp" (click)="updateParticipationFilter(FilterProp.ALL)" [value]="FilterProp.ALL" aria-label="Show all participations" /> <span class="ms-1" jhiTranslate="artemisApp.exercise.showAll"></span> </label> <!-- Similar changes for other radio inputs --> - </div> + </fieldset>
Line range hint
52-400: Improve consistency in button spacing and conditional checks.While the table structure is solid, there are a few areas that could be enhanced:
- Add consistent spacing classes to all button groups
- Simplify nested conditional checks for exercise group routing
For button groups, ensure consistent spacing:
- <div class="btn-group"> + <div class="btn-group me-2">For exercise group routing, consider simplifying:
- @if (exercise.isAtLeastInstructor) { - <div> - @if (!!exercise.exerciseGroup) { - <a routerLink="{{ value.id }}"> - {{ value.submissionCount }} - </a> - } - @if (!exercise.exerciseGroup) { - <a routerLink="{{ value.id }}/submissions"> - {{ value.submissionCount }} - </a> - } - </div> + @if (exercise.isAtLeastInstructor) { + <a [routerLink]="[value.id, exercise.exerciseGroup ? '' : 'submissions']"> + {{ value.submissionCount }} + </a>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/webapp/app/exercises/shared/participation/participation.component.html(1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/main/webapp/app/exercises/shared/participation/participation.component.html (1)
1-31: LGTM! Header layout improvements look good.
The new flex layout structure with d-flex flex-wrap align-items-center justify-content-between classes effectively centers the title and buttons. The use of @if directives follows the new Angular syntax guidelines.
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
laxerhd
force-pushed
the
bugfix/programming-exercises/misalignment-participationpage-buttons
branch
from
45f2fe7 to
9442db2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/webapp/app/exercises/shared/participation/participation.component.html (2)
32-51: Consider adding ARIA labels for better accessibility.While the layout is well-structured, the radio button group could benefit from improved accessibility.
Consider adding ARIA attributes:
- <div class="d-flex align-items-center mt-2"> + <div class="d-flex align-items-center mt-2" role="radiogroup" aria-label="Filter participations">
Line range hint
52-65: Consider adding trackBy function for better performance.The data table could benefit from a trackBy function to optimize rendering performance when the data changes.
Add a trackBy function to the component:
trackByParticipationId(index: number, participation: any): number { return participation.id; }Then update the table:
<jhi-data-table [isLoading]="isLoading" entityType="participation" [allEntities]="participations" + [trackBy]="trackByParticipationId" entitiesPerPageTranslation="artemisApp.exercise.resultsPerPage"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/exercises/shared/participation/participation.component.html(1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/main/webapp/app/exercises/shared/participation/participation.component.html (2)
1-31: LGTM! Header section layout improvements.
The changes effectively achieve the PR's objective of improving layout alignment. The use of flex classes (d-flex, align-items-center, justify-content-between) provides proper centering and spacing. The code correctly uses the new @if syntax as per guidelines.
Line range hint 52-400: LGTM! Consistent use of new Angular syntax.
The data table section correctly implements the new @if syntax throughout all conditional renderings, adhering to the coding guidelines.
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
I am closing this PR because it originates from a branch in a forked repository, and our test server cannot be deployed on branches from forked repositories. I have already transferred the changes to a new branch in the main repository and will open a new PR from there. |
Checklist
General
Client
Motivation and Context
Very optional changes, but it was a little offputting that the buttons weren't actually centered with anything.
(Another solution could be to center them with the original div structure to place them in the middle of the title and filters.)
Closes #9967.
Description
Steps for Testing
Prerequisites:
Instructor actions->ParticipationsTestserver 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
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Before:
After:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor