Skip to content
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

Update angular version to 15 and Keycloak to 13 #242

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

DominikVoigt
Copy link

This PR updates the Angular Version to 15 and the Angular Keycloak version to 13.

@DominikVoigt
Copy link
Author

We decided to stop at Angular version 15 instead of the current latest version of 16 due to certain dependencies that did not work with Angular version 16 at the time of writing:

  • @swimlane/ngx-graph Version: 8.0.3 (latest)
  • angular-bootstrap-md Version: 15.0.0 (latest)

Copy link
Contributor

@salmma salmma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bildschirmaufnahme.2023-10-25.um.15.56.40.mov

changing to other entities the page is not loading, see video. Note: navigation via menu on the right side works as usual
Getting error: ERROR Error: Uncaught (in promise): TypeError: e[a] is not a function

@salmma
Copy link
Contributor

salmma commented Oct 26, 2023

  • webpack is missing when using development debug mode in browser
  • this.router.navigate seems to fail when routing from one element view to another, see video for example

@buehlefs
Copy link
Contributor

buehlefs commented Oct 27, 2023

The .webpack dir and sourcemaps in general can be brought back by adding "sourceMap": true, in the default builder options in angular.json (e.g. line 25 "aot": true,).

From what I could tell from debugging, the error from the video is related to cleaning up the ripple effect of some buttons (e.g. the save button in the app-text-input for algorithm parameters and some add button(s)? that I have not investigated yet). This is likely because of some library update that causes problems (either because the library is incompatible with something or because we did not perform a required change in our code). I am not sure if that is the root cause for the navigation issue though.

Edit 31.10.: It is indeed the buttons that cause these problems. More specifically material icon buttons. Removing them from the page (or removing the mat-icon-button directive) "solves" the issue temporarily. But that is not really helpful with solving this issue. My hope is that an upgrade to angular 16 magically solves this problem (the two blocker dependencies should not be blocking as there are angular 16 compatible versions for them too, see my other comment about the dependencies). If that does not work we have two options: Replace all icon buttons with something different that works or properly investigate the issue (this could take a while...).

@@ -14,7 +14,7 @@ jobs:
- name: Set up Node.js
uses: actions/setup-node@v1
with:
node-version: 12
node-version: 16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test if this also works with node 18 and use 18 if possible (or even 20). Node version 16 is an lts version but is no longer maintained.

description: dialogResult.description,
};
const updatedComputeResourcePropertyType: ComputeResourcePropertyTypeDto =
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the usual style of brackets in the javascript/typescript world. This probably is related to the commented bracket rule in the linter config. If this is changed, then multiple other places that are also affected need to be changed as well.

@@ -8,11 +8,12 @@

}

/* TODO(mdc-migration): The following rule targets internal classes of form-field that may no longer apply for the MDC version. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

::ng-deep .mat-form-field-appearance-fill .mdc-text-field {
  background-color: transparent;
}

[matAutosizeMaxRows]="3"
[matAutosizeMinRows]="2"
[matTextareaAutosize]="true"
cdkAutosizeMaxRows="3"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a maximum of 3 rows seems a little too small for larger latex content this is intended for. Also the dialog renders way too thin...

@buehlefs
Copy link
Contributor

buehlefs commented Oct 27, 2023

About the dependencies blocking angular 16:

  • @swimlane/ngx-graph Version: 8.0.3 (latest) => The current latest version (8.2.2) should support angular 16
  • angular-bootstrap-md Version: 15.0.0 (latest) => If it is not too much effort this dependency could be ripped out entirely (aside from the breadcrumbs, everything I have found with a short search in the code should have a direct replacement with angular material components)

Edit 31.10.: angular-bootstrap-md has a direct successor that is compatible with angular 16: https://www.npmjs.com/package/mdb-angular-ui-kit The catch is that this uses a new Bootstrap version, so it may still be better/easier to remove the dependency entirely, as all styling issues would have to be checked either way. I have tested removing the dependency locally by force and it did not take too long to get it to compile again. But there were some style issues that would need fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants