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

HP-2047 New HDS into use #306

Merged
merged 10 commits into from
Feb 20, 2024
Merged

HP-2047 New HDS into use #306

merged 10 commits into from
Feb 20, 2024

Conversation

Riippi
Copy link
Contributor

@Riippi Riippi commented Feb 7, 2024

  • Update HDS to newest version and change components where it's necessary
  • Update translations and colors
  • Update some other dependencies
  • Add .nvmrc file

https://helsinkisolutionoffice.atlassian.net/browse/HP-2047

@Riippi Riippi requested review from NikoHelle and a team February 7, 2024 07:14
@codecov-commenter
Copy link

codecov-commenter commented Feb 7, 2024 •
edited
Loading

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (b4e531d) 83.96% compared to head (bd5141d) 83.83%.
Report is 1 commits behind head on main.

Files Patch % Lines
src/common/header/Header.tsx 50.00% 5 Missing and 3 partials ⚠️
src/common/footer/Footer.tsx 50.00% 0 Missing and 2 partials ⚠️
src/common/header/userDropdown/UserDropdown.tsx 50.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #306      +/-   ##
==========================================
- Coverage   83.96%   83.83%   -0.14%     
==========================================
  Files         170      168       -2     
  Lines        3824     3816       -8     
  Branches      884      881       -3     
==========================================
- Hits         3211     3199      -12     
- Misses        375      377       +2     
- Partials      238      240       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

NikoHelle
NikoHelle requested changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@NikoHelle NikoHelle left a comment

Choose a reason for hiding this comment

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

Nice upgrade!

Node version is also in Dockerfile, so that must also be upgraded to match other node versions.

The base-file used there, helsinkitest/node, should not be used, but ubi-images mentioned in https://helsinkicity.slack.com/archives/C01RNQ5PV5Z/p1702026731704299

The Dockerfile also uses ancient nginx 1.2. which could be upgraded too
https://helsinkicity.slack.com/archives/C01RNQ5PV5Z/p1707122854109759

.github/workflows/CI.yml Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
@NikoHelle
Copy link
Contributor

In my Chrome the app has weird borders and always visible scrollbar

Local:
image

In dev:
image

There is some css missing, as the body has default 8px margin. That is overridden in dev-version.

Could be the problem that Tuomo has been fixing lately.

@NikoHelle
Copy link
Contributor

Clicking the internal link "Evästeasetukset" reloads the whole app again. That is prevented in the dev version (old footer)

@NikoHelle
Copy link
Contributor

Clicking the menu in mobile view does nothing.

@NikoHelle
Copy link
Contributor

Is there is a reason why English is the first primary language? Finnish is most likely the most used language.

NikoHelle
NikoHelle reviewed Feb 9, 2024
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
NikoHelle
NikoHelle requested changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@NikoHelle NikoHelle left a comment

Choose a reason for hiding this comment

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

Few UI bugs should be fixed...

@terovirtanen
Copy link
Contributor

HELSINKI-PROFILE-UI branch is deployed to platta: https://helsinki-profile-pr306-ui.dev.hel.ninja 🚀🚀🚀

1 similar comment
@terovirtanen
Copy link
Contributor

HELSINKI-PROFILE-UI branch is deployed to platta: https://helsinki-profile-pr306-ui.dev.hel.ninja 🚀🚀🚀

@Riippi
Copy link
Contributor Author

Riippi commented Feb 14, 2024

Few UI bugs should be fixed...

  • 8px padding was becaue hds removed normalise. Now this is fixed in css
  • "Evästeasetukset" link is now router link and doesn't reload whole page. Also "saatavuusseloste" fixed.
  • mobile menu button is fixed
  • Finnish is now default language and automatic browser check is removed for now

@Riippi Riippi force-pushed the HP-2047-new-hds-into-use branch 2 times, most recently from 336be8d to ee26b29 Compare February 15, 2024 14:47
@Riippi
Copy link
Contributor Author

Riippi commented Feb 15, 2024

Added test for header language change so the test coverage is over 65%

@terovirtanen
Copy link
Contributor

HELSINKI-PROFILE-UI branch is deployed to platta: https://helsinki-profile-pr306-ui.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

HELSINKI-PROFILE-UI branch is deployed to platta: https://helsinki-profile-pr306-ui.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

HELSINKI-PROFILE-UI branch is deployed to platta: https://helsinki-profile-pr306-ui.dev.hel.ninja 🚀🚀🚀

@NikoHelle
Copy link
Contributor

Good fixes! Couple of issues

The ingress text should not be bold according to designs:

image

Finnish should be the first language option
image

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Feb 20, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue

Measures
0 Security Hotspots
76.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@terovirtanen
Copy link
Contributor

HELSINKI-PROFILE-UI branch is deployed to platta: https://helsinki-profile-pr306-ui.dev.hel.ninja 🚀🚀🚀

NikoHelle
NikoHelle approved these changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@NikoHelle NikoHelle left a comment

Choose a reason for hiding this comment

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

All done!

Something I forget maybe 50% time when updating: set new version to package.json!

It will show in https://profiili.dev.hel.ninja/env-config.js and then you can check what version each env is running.

You can freely update it to any version number you want, nothing is depending on it. :)

@Riippi Riippi merged commit dfccc07 into main Feb 20, 2024
20 checks passed
@Riippi Riippi deleted the HP-2047-new-hds-into-use branch February 20, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NikoHelle NikoHelle NikoHelle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Riippi @codecov-commenter @NikoHelle @terovirtanen @mrTuomoK