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.

Sign up for GitHub

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

Jump to bottom

Feature/#101 angular15 update #113

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Feature/#101 angular15 update #113

wants to merge 20 commits into from

Conversation

fabioformosa
Copy link
Owner

No description provided.

midhunadarvin and others added 3 commits February 2, 2024 16:05
@midhunadarvin
feat: update angular version from 14 to 15
2105e28 
- update package.json dependencies
- npm audit force fix
- add `MatDialogModule` to app module imports
- mat-form-field has appearance property `outline` and `fill` now
- update unit tests
@fabioformosa
Merge branch 'feature/#101_angular15_update' into feat/angular-update
b294273
@fabioformosa
Merge pull request #111 from midhunadarvin/feat/angular-update
1421c52 
feat: update angular version from 14 to 15
@fabioformosa fabioformosa mentioned this pull request Feb 2, 2024
Merged
@fabioformosa
Copy link
Owner Author

Some style regressions prevent to merge in the develop branch:

  • input field style
  • missed padding
  • fonts.

I leave the PR opened.

@fabioformosa fabioformosa mentioned this pull request Feb 2, 2024
Open
@midhunadarvin
feat: update padding & styles
c751905 
- apply padding fixes
- remove `enableTrigger` variable and directly use `simpleTriggerReactiveForm.enabled`
-  apply default styles of matInput
@midhunadarvin midhunadarvin mentioned this pull request Feb 3, 2024
Merged
@fabioformosa
Copy link
Owner Author

fabioformosa commented Feb 8, 2024 •
edited
Loading

Hi @midhunadarvin,

Observations

  • OK for the new form field appearence defined by the material
  • I totally agree with migrating from the deprecated fxFlex to tailwind
  • There are some mismatches and regressions

Style Mismatches

Font mismatch

Even though in CSS the family font seems unchanged, actually it looks like that the font family applied is different
image
image

Scrolling (regression)

In the prev version, when logs reach the bottom of the window, the log section scrolls while the window remains fixed.
In the new version, the window scrolls too.
image
image

Font size mismatch

image
image
image
image

Editable trigger name (regression)

In the prev version, in case of rescheduling, the trigger name was immutable.
In the new version, it's mutable.

image
image

[minor] mismatching of the padding in the dialog

image
image

@midhunadarvin
fix: style regression issues
adb4864 
- add Roboto font face as dependency
- fix font size regression issue in select inputs
- fix scroll regression issue
- fix triggerName not disabled during reschedule
@midhunadarvin midhunadarvin mentioned this pull request Feb 9, 2024
Merged
@midhunadarvin
Copy link
Contributor

Hi @fabioformosa

  1. Font mismatch : Even though the font-family was given Roboto, Helvetica Neue, the fonts are not included either via build or cdn. I could see that on my machine it had font Helvetica Neue so it defaulted to it. If i am just using Roboto it defaulted to Times new roman as i did not have the font installed on my machine.

I have included Roboto fontface as npm dependency and included it in the build.

  1. Scrolling (regression) : It is fixed, please check.
  2. Font size mismatch : It is fixed, please check.
  3. Editable trigger name (regression) : It is fixed, but during testing i could find another issue. If a trigger job has been executed, rescheduling it causes a bug. I think it is because the trigger ( without repeat count ) is deleted from db by quartz scheduler.
  4. [minor] mismatching of the padding in the dialog : it is fixed, please check.

fabioformosa and others added 4 commits February 19, 2024 23:34
@fabioformosa
Merge pull request #121 from ChistaDATA/feature/#101_angular15_update
baa01d1 
fix: style regression issues
@midhunadarvin @fabioformosa
feat: update angular version from 14 to 15
2debf6c 
- update package.json dependencies
- npm audit force fix
- add `MatDialogModule` to app module imports
- mat-form-field has appearance property `outline` and `fill` now
- update unit tests
@midhunadarvin @fabioformosa
feat: update padding & styles
bc0619a 
- apply padding fixes
- remove `enableTrigger` variable and directly use `simpleTriggerReactiveForm.enabled`
-  apply default styles of matInput
@midhunadarvin @fabioformosa
fix: style regression issues
7cef355 
- add Roboto font face as dependency
- fix font size regression issue in select inputs
- fix scroll regression issue
- fix triggerName not disabled during reschedule
@midhunadarvin
Copy link
Contributor

Hi @fabioformosa Anything pending on this PR ? Are we okay to merge this to develop ?

@fabioformosa
Copy link
Owner Author

There's still the font size mismatch.
Apart from that, the angular team has released the last version of the flex-layout lib for angular 15 (https://www.npmjs.com/package/@angular/flex-layout). It should be the last call before closing the repo.
Since the pure CSS is a temporary solution in the path towards tailwind and it introduced some style issues, I was thinking whether it's worth to upgrade flex-layout one last time and embrace directly tailwind preparing for angular 17.

@midhunadarvin
Copy link
Contributor

In this PR, the latest version of @angular/flex-layout 15.0.0-beta.42 has been used. The style issues introduced were mainly due to the angular material library update.

I think reverting to flex-layout doesn't help much in the short run as it would be easier to migrate to tailwind from pure css rather than from flex-layout. So removing dependency from flex-layout little by little is advised, since flex-layout is deprecated.

@fabioformosa
Copy link
Owner Author

ok, it makes sense.

@midhunadarvin
Copy link
Contributor

Hi @fabioformosa I have fixed the footer content font-size in #122

@fabioformosa
Merge branch 'feature/#101_angular15_update' of github.com:fabioformo…
f96e356 
…sa/quartz-manager into feature/#101_angular15_update

# Conflicts:
#	.gitignore
#	quartz-manager-frontend/src/styles.css
@fabioformosa
#101 added smaller fonts for job class and misfire instructions
6ec8866
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fabioformosa @midhunadarvin