-
Notifications
You must be signed in to change notification settings - Fork 12
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
OEL-2166: Upgrade to BCL 1.0.0 #267
Conversation
5081ccd
to
da96cc4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also noticed inpage navigation is not working.
And while testing in showcase I noticed the offcanvas bit in the search form is visible only in mobile.
kits/default/package.json
Outdated
@@ -13,7 +13,7 @@ | |||
"production": "npm-run-all build:*" | |||
}, | |||
"devDependencies": { | |||
"@openeuropa/bcl-builder": "0.24.1", | |||
"@openeuropa/bcl-builder": "0.25.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets use the same 28.1
docker-compose.yml
Outdated
@@ -28,7 +28,7 @@ services: | |||
user: "node" | |||
working_dir: /home/node/app | |||
environment: | |||
- NODE_ENV=${NODE_ENV} | |||
- NODE_ENV=${NODE_ENV:-''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give me a bit on info, the commit message should also be a bit more descriptive.
:-
evaluates to use ''
if NODE_ENV is unset or empty, but we give an empty string as default, would -
(use '' if NODE_ENV is unset) work ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll replace with -
only as it's the same operation done by Docker: provides an empty string as fallback only when the variable is not defined at all.
Fixed the offcanvas, for which I reviewed the updates back in the days but forgot to bring them. The inpage navigation is broken so I'll create an issue for it. |
…ate search variant.
Bootstrap 5.2 has bugs in the scrollspy script. One of the needed fixes is to not target the body element but an inner wrapper as element to track. Unfortunately this doesn't seem to be enough either.
No description provided.