General: Adapt exercise footer for small screens and general footer for environments
#8359
Conversation
WalkthroughThe update entails an extensive revamp to enrich the user interface, accessibility, and overall user experience of the web application. These enhancements cover various components, including CSS adjustments for responsive layouts, HTML template updates for conditional rendering and dynamic class assignments, and TypeScript modifications for improved functionality and code clarity. The primary aim is to streamline user interactions and establish a more intuitive and efficient application environment. Changes
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (4)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
the
client
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| height: calc(100vh - $footer-height-dev - $header-height); | ||
| height: calc(100vh - var(--sidebar-footer-height-dev) - $header-height); | ||
| } | ||
| @media (max-width: 768px) { |
There was a problem hiding this comment.
Expected empty line before at-rule
According to CSS best practices, it's recommended to have an empty line before at-rule declarations for better readability. Adding an empty line before the @media rule at line 223 will align with these practices and improve the code's readability.
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| @media (max-width: 768px) { | |
| @media (max-width: 768px) { |
| height: calc(100vh - $footer-height-dev); | ||
| height: calc(100vh - var(--sidebar-footer-height-dev)); | ||
| } | ||
| @media (max-width: 768px) { |
There was a problem hiding this comment.
Expected empty line before at-rule
For consistency and readability, it's recommended to add an empty line before the @media rule at line 32. This adjustment aligns with CSS best practices and improves the code's readability.
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| @media (max-width: 768px) { | |
| @media (max-width: 768px) { |
| @@ -11,7 +12,7 @@ describe('ExerciseUpdateNotificationComponent', () => { | |||
| beforeEach(() => { | |||
| TestBed.configureTestingModule({ | |||
| imports: [MockModule(FormsModule)], | |||
| declarations: [ExerciseUpdateNotificationComponent], | |||
| declarations: [ExerciseUpdateNotificationComponent, MockPipe(ArtemisTranslatePipe)], | |||
There was a problem hiding this comment.
Ensure high test coverage by adding more test cases.
Would you like suggestions for additional test cases to cover other functionalities of the ExerciseUpdateNotificationComponent?
src/test/javascript/spec/component/forms/form-footer.component.spec.ts
Outdated
Show resolved
Hide resolved
| describe('Git Information', () => { | ||
| it('should display git information if not in production or on a test server', () => { | ||
| component.isProduction = false; | ||
| component.isTestServer = true; | ||
| fixture.detectChanges(); | ||
|
|
||
| const gitInfoElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper'); | ||
| expect(gitInfoElement).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
Add tests for git information display logic.
The test for git information display based on environment flags is good. However, consider adding more edge cases, such as when both isProduction and isTestServer are true, to ensure comprehensive coverage. Would you like assistance in implementing these tests?
| describe('Git Information Detailed Testing', () => { | ||
| beforeEach(() => { | ||
| component.gitBranch = 'main'; | ||
| component.gitCommitId = 'abc123'; | ||
| component.gitTimestamp = '2023-04-01T12:00:00Z'; | ||
| component.gitCommitUser = 'user123'; | ||
| component.isProduction = false; | ||
| component.isTestServer = false; | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it('should display all git information', () => { | ||
| const gitBranchElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(1)'); | ||
| const gitCommitElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(2)'); | ||
| const gitTimestampElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(3)'); | ||
| const gitUserElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(4)'); | ||
|
|
||
| expect(gitBranchElement.textContent).toContain('main'); | ||
| expect(gitCommitElement.textContent).toContain('abc123'); | ||
| expect(gitTimestampElement.textContent).toContain('2023-04-01T12:00:00Z'); | ||
| expect(gitUserElement.textContent).toContain('user123'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Refine git information detailed testing for readability and maintainability.
The detailed git information tests are well-structured, but they could be made more readable and maintainable by abstracting repetitive query selectors into a helper function. This would make the tests cleaner and easier to update in the future.
+ function queryGitInfo(selector) {
+ return fixture.debugElement.nativeElement.querySelector(selector);
+ }
- const gitBranchElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(1)');
+ const gitBranchElement = queryGitInfo('.footer-git-wrapper .footer-git:nth-child(1)');Repeat this pattern for the other git information elements.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| describe('Git Information Detailed Testing', () => { | |
| beforeEach(() => { | |
| component.gitBranch = 'main'; | |
| component.gitCommitId = 'abc123'; | |
| component.gitTimestamp = '2023-04-01T12:00:00Z'; | |
| component.gitCommitUser = 'user123'; | |
| component.isProduction = false; | |
| component.isTestServer = false; | |
| fixture.detectChanges(); | |
| }); | |
| it('should display all git information', () => { | |
| const gitBranchElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(1)'); | |
| const gitCommitElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(2)'); | |
| const gitTimestampElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(3)'); | |
| const gitUserElement = fixture.debugElement.nativeElement.querySelector('.footer-git-wrapper .footer-git:nth-child(4)'); | |
| expect(gitBranchElement.textContent).toContain('main'); | |
| expect(gitCommitElement.textContent).toContain('abc123'); | |
| expect(gitTimestampElement.textContent).toContain('2023-04-01T12:00:00Z'); | |
| expect(gitUserElement.textContent).toContain('user123'); | |
| }); | |
| }); | |
| describe('Git Information Detailed Testing', () => { | |
| beforeEach(() => { | |
| component.gitBranch = 'main'; | |
| component.gitCommitId = 'abc123'; | |
| component.gitTimestamp = '2023-04-01T12:00:00Z'; | |
| component.gitCommitUser = 'user123'; | |
| component.isProduction = false; | |
| component.isTestServer = false; | |
| fixture.detectChanges(); | |
| }); | |
| function queryGitInfo(selector) { | |
| return fixture.debugElement.nativeElement.querySelector(selector); | |
| } | |
| it('should display all git information', () => { | |
| const gitBranchElement = queryGitInfo('.footer-git-wrapper .footer-git:nth-child(1)'); | |
| const gitCommitElement = queryGitInfo('.footer-git-wrapper .footer-git:nth-child(2)'); | |
| const gitTimestampElement = queryGitInfo('.footer-git-wrapper .footer-git:nth-child(3)'); | |
| const gitUserElement = queryGitInfo('.footer-git-wrapper .footer-git:nth-child(4)'); | |
| expect(gitBranchElement.textContent).toContain('main'); | |
| expect(gitCommitElement.textContent).toContain('abc123'); | |
| expect(gitTimestampElement.textContent).toContain('2023-04-01T12:00:00Z'); | |
| expect(gitUserElement.textContent).toContain('user123'); | |
| }); | |
| }); |
krusche
changed the title
General: Adapt footer for small screens and environmentsGeneral: Adapt exercise footer for small screens and environments
rabeatwork
changed the title
General: Adapt exercise footer for small screens and environmentsGeneral: Adapt exercise footer for small screens and general footer for environments
muradium
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
muradium
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
GitHub Actions
Inactive
src/main/webapp/app/exercises/quiz/participate/quiz-participation.component.scss
Outdated
Show resolved
Hide resolved
Save icon does not adapt to page zoom anymore. Also the task green ticks stay the same size and go off the center, if that's related. |
@muradium Which browser do you use? |
Checklist
General
Client
Motivation and Context
Especially for small screens, it's important to have a compressed footer and not to waste any space.
Description
The footer in the exercises was partially very high. The footer height has been reduced overall, and for small screens, the buttons are turned into icon buttons. Also, the general Artemis footer was set to 4rem by default. That's fine for development but too high for production. This has been adjusted as well.
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Screenshots