-
Notifications
You must be signed in to change notification settings - Fork 5
feat(Site): Remove unit library from teacher home #2162
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
Conversation
- Redirect /teacher/home/schedule to /teacher/home - Redirect /teacher/home/library/* to /curriculum - Always show site header links when logged in as teacher
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.
Pull Request Overview
This PR removes the teacher unit library feature from the teacher home screen while updating related routing and header display logic. Key changes include the deletion of teacher and community library components and their tests, updates to template path references for official and community library details, and adjustments to header links and curriculum button styling.
Reviewed Changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/modules/library/teacher-project-library/teacher-project-library.component.ts | Removed teacher project library component logic and related code. |
| src/app/modules/library/teacher-project-library/teacher-project-library.component.spec.ts | Removed tests for the teacher project library component. |
| src/app/modules/library/public-unit-type-selector/public-unit-type-selector.component.ts | Updated template URL references to match the new file structure. |
| src/app/modules/library/public-library/public-library.component.ts | Removed the getDetailsComponent method as part of the cleanup. |
| src/app/modules/library/official-library/official-library.component.ts | Removed details component logic no longer needed for the updated design. |
| src/app/modules/library/official-library/official-library.component.html | Simplified template by removing unused ng-template wrapper. |
| src/app/modules/library/library/library.component.ts | Cleaned up unused abstract methods and helper functions related to details dialogs. |
| src/app/modules/library/library.module.ts | Removed declarations and imports for teacher and community library components. |
| src/app/modules/library/community-library/community-library.component.ts & .spec.ts | Removed entire community library component and test cases. |
| src/app/modules/header/header-links/header-links.component.spec.ts and .html | Updated header links test and template to adjust behavior based on user role using Angular 17 control flow. |
| src/app/curriculum/curriculum.component.html | Changed button style from mat‑flat‑button to mat‑raised‑button. |
Files not reviewed (3)
- src/app/modules/library/community-library/community-library.component.html: Language not supported
- src/app/modules/library/teacher-project-library/teacher-project-library.component.html: Language not supported
- src/app/modules/library/teacher-project-library/teacher-project-library.component.scss: Language not supported
In future, we should add /curriculum/personal route and redirect there.
Remove unnecessary SASS imports.
- Add icon to Class Schedule. - Increase font-weight of teacher home welcome message.
hirokiterashima
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.
Looking great. 👍
I realized that we actually need to implement the /curriculum/personal path in this PR, and go there after copying a unit and flash/animate the newly copied unit to draw attention to it, as we did before. Right now, when we copy a Public unit, the user won't see any visual queues that the project has been copied into My Units.
We should also fix the issue where the thumbnails for My Units appear as gray boxes before this PR gets merged into develop. I'll look into this.
Switch to mat-tab-nav-bar for toggling between public and personal library.
|
Changes look great. I really like the new icons and ability to toggle between class schedule and unit library. I noticed that copying run unit takes me to My Units, but it doesn’t highlight/flash the new unit. It works when I copy a unit that is in the "Public" or "My Units". It would be nice to draw attention to a new unit, but if it's not an easy fix, let's not sweat it. Also, it looks like we can now remove PersonalLibraryDetailsComponent and the dialog dependency from LibraryComponent and its child components? |
…g highlighted in Personal Library.
|
I found a new issue: I can't set up a run from the Library. When I click on "use with class", I see the "Use with Class" dialog but the body is empty, and there's an error in the js console: Strange... @Aaron-Detre and I saw this error before, and we fixed it by adding |
|
When I cleared out my .angular/cache and node_modules folder and restarted the server, the seems to have fixed itself. |
|
🎉 This PR is included in version 5.181.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Changes
Remove unit library from teacher home.
/teacher/home/scheduleto/teacher/home./teacher/home/library/* to/curriculum.Test