Conversation
jr-rk
commented
Sep 29, 2025
jr-rk
commented
| Phases | MP | MM | MB | MR | JM | Total |
|---|---|---|---|---|---|---|
| ETA | 0 | 0 | 0 | 0 | 0 | 0 |
| Developing | 0 | 0 | 0 | 0 | 0 | 0 |
| Review | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | - | - | - | - | - | 0 |
| ETA est. | 0 | |||||
| ETA cust. | - | - | - | - | - | 0 |
There was a problem hiding this comment.
Pull Request Overview
This PR implements significant UI customization and theme changes for a MENDELU (Mendel University) DSpace instance, transforming it from the default CLARIN theme to a MENDELU-branded theme.
- Updates branding elements throughout the application to use MENDELU logos and styling
- Implements new color scheme using cyan/teal colors (#00a499) as the primary brand color
- Creates new custom theme components and overrides existing components to match MENDELU requirements
Reviewed Changes
Copilot reviewed 100 out of 108 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/themes/eager-themes.module.ts | Adds duplicate import and includes additional EagerThemeModule |
| src/themes/dspace/styles/_mendelu_sass_variable_overrides.scss | Introduces MENDELU brand color variables |
| Multiple theme files | Comprehensive theming updates replacing CLARIN branding with MENDELU |
| src/app/shared/shared.module.ts | Adds new badge components for Dimensions and Altmetric |
| config/config.yml | Configures custom theme with favicon settings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import { EagerThemeModule as DSpaceEagerThemeModule } from './dspace/eager-theme.module'; | ||
| import { EagerThemeModule } from './dspace/eager-theme.module'; |
There was a problem hiding this comment.
Duplicate import detected. The second import of EagerThemeModule from the same path will override the first aliased import DSpaceEagerThemeModule, potentially causing issues. Remove the duplicate import on line 3.
| imports: [ | ||
| DSpaceEagerThemeModule, | ||
| // CustomEagerThemeModule, | ||
| // Uncomment this because the `untyped-item` theming is not working when it is commented out. | ||
| // Issue: https://github.com/DSpace/dspace-angular/issues/1897 | ||
| // Useful info in PR: https://github.com/DSpace/dspace-angular/pull/2262#issuecomment-1557146081 | ||
| EagerThemeModule, | ||
| ], |
There was a problem hiding this comment.
Due to the duplicate import issue, EagerThemeModule is the same as DSpaceEagerThemeModule, making this import redundant and potentially causing circular dependency issues. This should be resolved by fixing the import statements.
| &:hover, &:focus { | ||
| color: var(--ds-header-icon-color-hover); | ||
| background-color: white; | ||
| color: var(--lt-mendelu-green); |
There was a problem hiding this comment.
CSS variable --lt-mendelu-green is referenced but not defined anywhere in the codebase. This should likely be --lt-vsb-tuo-cyan or the MENDELU equivalent color variable $cyan: #00a499 that was defined.
| color: var(--lt-mendelu-green); | |
| color: #00a499; |
| &:hover, &:focus { | ||
| color: var(--ds-header-icon-color-hover); | ||
| background-color: white; | ||
| color: var(--lt-mendelu-green); |
There was a problem hiding this comment.
CSS variable --lt-mendelu-green is undefined. This should be updated to use the correct MENDELU brand color variable.
| color: var(--lt-mendelu-green); | |
| color: #009639; // MENDELU brand green |
Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk>
…ms, logos ratio, contact us page, disabled detailed list view in my-dspace, clickable rows as block in admin-sidebar
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 116 out of 138 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (window['__dimensions_embed']) { | ||
| setTimeout(() => window['__dimensions_embed'].addBadges(), 500); | ||
| } |
There was a problem hiding this comment.
[nitpick] Using bracket notation to access window properties bypasses TypeScript type checking. Consider declaring the property on the Window interface or using a more type-safe approach.
| if (window['_altmetric_embed_init']) { | ||
| setTimeout(() => window['_altmetric_embed_init'](), 500); | ||
| } |
There was a problem hiding this comment.
[nitpick] Same issue as dimensions badge - using bracket notation to access window properties bypasses TypeScript type checking. Consider declaring the property on the Window interface.
| </a> | ||
| <a href="https://www.clarin.eu/"> | ||
| <img class="clarin-logo" src="assets/images/clarin-logo.svg" alt="Clarin logo" > | ||
| <img src="assets/images/mendelu-logo.svg" [attr.alt]="'menu.header.image.logo' | translate"/><img class="clarin-logo" src="assets/images/clarin-logo.svg" alt="Clarin logo" > |
There was a problem hiding this comment.
Two image tags are concatenated without proper spacing or structure. The CLARIN logo appears to be leftover from the old theme and should likely be removed for MENDELU branding.
| <img src="assets/images/mendelu-logo.svg" [attr.alt]="'menu.header.image.logo' | translate"/><img class="clarin-logo" src="assets/images/clarin-logo.svg" alt="Clarin logo" > | |
| <img src="assets/images/mendelu-logo.svg" [attr.alt]="'menu.header.image.logo' | translate"/> |
|
Closing because it will be customized in the DSpace9 |
