Update for node iron lts #155
Conversation
|
Hello @mattgoud! This is your first pull request on classic-theme repository of the PrestaShop project. Thank you, and welcome to this Open Source community! |
.hintrc
Outdated
There was a problem hiding this comment.
Shouldn't this be put in the _dev folder along with the package.json file and all the other configs?
_dev/css/components/products.scss
Outdated
| @@ -150,8 +150,6 @@ | |||
| width: auto; | |||
|
|
|||
| .btn { | |||
| padding: 0.5rem 0.6875rem; | |||
There was a problem hiding this comment.
Why remove this one? This seems like an UX change not required for the Node 20 compatibility no?
Unless this is related to the bootstrap-touchspin update you mentioned?
_dev/css/partials/_commons.scss
Outdated
| } | ||
|
|
||
| .ui-autocomplete.ui-front { | ||
| z-index: 999; | ||
| } | ||
|
|
||
| .btn.btn-touchspin.js-touchspin.bootstrap-touchspin-up { | ||
| border-radius: 0; |
There was a problem hiding this comment.
Same question as above, what is the link between this and the Node 20 compatibility?
_dev/js/cart.js
Outdated
| @@ -72,8 +72,8 @@ function createSpin() { | |||
| $.each($(spinnerSelector), (index, spinner) => { | |||
| $(spinner).TouchSpin({ | |||
| verticalbuttons: true, | |||
| verticalupclass: 'material-icons touchspin-up', | |||
| verticaldownclass: 'material-icons touchspin-down', | |||
| verticalupclass: 'glyphicon glyphicon-plus', | |||
There was a problem hiding this comment.
Doesn't this change the style of all the icons?
Is it also because of bootstrap-touchspin update?
_dev/package.json
Outdated
| "css-loader": "^6.7.1", | ||
| "csso-webpack-plugin": "^2.0.0-beta.3", | ||
| "esbuild-loader": "^2.15.1", | ||
| "bootstrap-touchspin": "^4.7.3", |
There was a problem hiding this comment.
Ok so bootstrap version was not changed, which is critical here for backward compatibility of all the modules.
I wonder if bootstrap-touchspin should be updated. For the same reason, updating to a new major version maybe bring some breaking changes. Is it possible to keep the same major version? Or was it not possible because of Node 20 update?
florine2623
added
QA ✔️
|
No need functional testing :) ok to be merged |
|
PR merged, well done! Message to @PrestaShop/committers: do not forget to milestone it before the merge. |
|
Thanks @mattgoud |
Upgrade node modules
(see #37041 )