Closed ValentinFutterer closed 2 months ago
Issues
3 New issues
0 Accepted issues
Measures
0 Security Hotspots
80.0% Coverage on New Code
0.4% Duplication on New Code
@ValentinFutterer I've been reviewing the PR, and so far, its great work...I know it was a very big one. However, there are some issues with the test cases. Would you be so kind as to resolve these issues? You can run the ci:text
script to see where the problems occur.
Description
Refactoring/Config
What does this PR do?
Updated Angular from 14 to 17 - followed the official Angular update Guide. Used
npx npm-check-updates -u
to update package.json. Updated the config by deleting the old project.json's and runningnx g @nrwl/workspace:convert-to-nx-project --all
. This replaced the angular.json with two project.json files, since angular.json is no longer supported with nx. I also rannx repair
.After checking all the boxes in the Angular update Guide, I updated the components. Angular offers a Migration Tool, which takes over some of the component migration. Most of the components received just small changes in their attributes, inputs or they got different containers. Most components got visual changes. I tried to recreate the original DAMAP as much as possible, but some components changed too much.
mat-card got big changes and can not be used like we did, see the project component on chosen project. I found a hacky solution, which kind of recreates the original design. See old vs new.
The main problems with the new component are firstly, that all new divs are put into a new line automatically and secondly,
mat-card
does not support buttons/icons anywhere on a card except the bottom left.Another similar problem exists within the mat-list-option component, but not as bad. The solution I came up with is still noticeable different from the original, plus it is still very hacky.
The language button in the navbar (layout component) did not take styling from surrounding css - needed its own class. But with its own css class, it was still not possible to change its color, I needed inline css to accomplish this. It is possible with other buttons though. I have no idea where this issue stems from. Possibly related to https://github.com/angular/components/issues/26153
Globally some text font-sizes and font.styles have changed - for example buttons have a different style, hints and placeholders in inputs have a different size ect. Some text is also now greyed out. Input fields have a gray background:
mat-tab
has a scrolling bar, which can not be removed it seems. Icon button alignment changed at some places due to different positioning. icons inside buttons with text got smaller. Titles in mat-cards look like normal text for some reason, this shouldnt be the case.Also, I was not able to visually check access.component, repo-recommendation.component and the storage.component, since the backend mocks to not allow this currently.
Code review focus
There are still inconsistencies compared to how it was before plus the fonts are not styled at many places. I left it as is for now, since we are going to redo the whole frontend anyways. So please look over the the configs and doublecheck, if everything important still works the same. Functionality of all components should still be there, but please check this as well.
File change breakdown
apps/damap-frontend/src/app/app.module.ts
libs/damap/src/lib/components/dmp/dmp.module.ts
Upgraded HttpClientModule, see their github.
apps/damap-frontend/src/app/components/layout/layout.component.html
See the paragraph about the language button above - visually it is the same, but the code is now pretty ugly. h1 tag does not effect text inside mat-toolbar-row anymore - strong still works so i just replaced it. See old vs new:
libs/damap/src/lib/components/dmp/costs/costs.component.css
Radio buttons have margin on their own now. In fact, the newer version has more space between the buttons, without setting a margin.
libs/damap/src/lib/components/dmp/data-storage/external-storage/external-storage.component.html
mat-card-title
isnt in bold anymore, looks the same as subtitle. This shouldnt be the case at all, on the angular website, the title works correctly, see this stackblitzmat-card-header
is needed now as a container for title.libs/damap/src/lib/components/dmp/legal-ethical-aspects/legal-ethical-aspects.component.css
Removed margin since radio buttons have their own now. Standard margin is greater than the one that was given manually.
libs/damap/src/lib/components/dmp/licenses/licenses.component.html
No visual changes, except the changed fonts, greyed input and padding inside input. Those happened in the whole project though.
libs/damap/src/lib/components/dmp/people/confirm-deletion-dialog.html
Is this even used anywhere?.
libs/damap/src/lib/components/dmp/people/contributor-manual/contributor-manual.component.html
New visual comes from the new look of components, did not happen through my changes.
libs/damap/src/lib/components/dmp/people/people.component.html
Visually the same, except the input fields, which look different in the whole project.
libs/damap/src/lib/components/dmp/project/project-list/project-list.component.html
See above for what changed.
libs/damap/src/lib/components/dmp/project/project.component.html
See above for what changed with the project cards. Tabs now take over the whole width and have a scrollbar that cant be removed.
libs/damap/src/lib/components/dmp/specify-data/data-mc/data-mc.component.html
The class isnt needed anymore - same as with the other radio buttons.
libs/damap/src/lib/components/dmp/specify-data/dataset-dialog/dataset-dialog.component.html
Buttons have different padding since now they are inside the
mat-dialog-actions
tag. The title isnt influenced by theh1
tag anymore, for some reason. Input fields look different - same for whole project.libs/damap/src/lib/components/dmp/specify-data/dataset-table/dataset-table.component.html
Changed button with just icon to icon button, now has the round icon button appearance when hovering.
libs/damap/src/lib/widgets/delete-warning-dialog/delete-warning-dialog.component.ts
I dont think this component is used anywhere.
libs/damap/src/lib/widgets/dmp-table/dmp-table.component.html
Adapt to the other paginator.
libs/damap/src/lib/widgets/env-banner/env-banner.component.html
Title doesnt get any styling by default (for some reason) and the h1 tag doesnt work either - used string for similar effect. Something is wrong with
mat-card-title
in this project.libs/damap/src/lib/widgets/export-warning-dialog/export-warning-dialog.html
Tite gets no styling (again).
libs/damap/src/lib/widgets/license-wizard/license-wizard-dialog.html
For some reason, the
h3
tags are now being rendered, causing the license name and the question "What do you want to deposit" to look bigger. But the dialog title gets no styling now, for some reason.libs/damap/src/lib/widgets/person-card/person-card.component.html
No visual changes except buttons are now icon buttons.
libs/damap/src/lib/widgets/tree-select-form-field/tree-select-form-field.component.html
Basically act the same, except different outline when selected.
Checks
closes GH-145