-
Notifications
You must be signed in to change notification settings - Fork 23
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
Implemention of new WebComponent header #663
Conversation
js/plugins/Header.jsx
Outdated
const georHeader = document.createElement("geor-header"); | ||
const georHeaderScript = document.createElement("script"); |
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.
Just wondering if we couldn't put these elements in the index.html
and then just set the script.src and attributes here,... also to prevent the check for multiple header creation above. But I guess this does not 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.
I modify it :)
js/plugins/Header.jsx
Outdated
const georHeaderScript = document.createElement("script"); | ||
|
||
georHeader.setAttribute("active-app", page); | ||
georHeader.setAttribute("legacy-header", false); |
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.
Is this line necessary, to set the legacy-header
equals false
before the georHeader.setAttribute("legacy-header", legacy);
below?
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.
It isn't, just forgot to remove it.
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.
Thanks for the integration and the latest adaptions @f-necas ! I wasn't able to test it on my side, but the changes LGTM.
Also tested last week and it worked :-) |
georchestra/georchestra#4065
Implements the new georchestra/header in geoserver.
Main PR : georchestra/georchestra#4065