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.

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

Webcomponent header #8

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Webcomponent header #8

merged 1 commit into from
Dec 11, 2023

Conversation

f-necas
Copy link

@f-necas f-necas commented Dec 1, 2023

Webcomponent header

Needs resolution of PR :

Implements the new georchestra/header in datafeeder-ui.
Main PR :

Changes

Header script

Old iframe header is removed from the image and can be available while with an sh script (e.g. use iframe header):

DATAFEEDER_DIR=${1:-/usr/share/nginx/html/}
SNIPPET="<script src='https://cdn.jsdelivr.net/gh/georchestra/header@dist/header.js'></script><geor-header active-app='import' style='height:90px' legacy-header="true" legacy-url="/header/"></geor-header>"

if grep -q "${SNIPPET}" "${DATAFEEDER_DIR}/index.html"; then
  echo "[INFO] geOrchestra: header already present."
  exit 0
fi

echo "[INFO] geOrchestra: adding header in the main page..."
sed -i "s#<body class=\"m-0 h-full flex flex-col\">#<body class=\"m-0 h-full flex flex-col\">${SNIPPET}#" ${DATAFEEDER_DIR}/index.html

For all informations about the header : see README.md

@f-necas f-necas requested a review from tkohr December 1, 2023 13:27
Copy link

@tkohr tkohr left a comment

Choose a reason for hiding this comment

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

Thanks @f-necas ! Just left a question for clarification.

And I guess there will be another linked PR to the datadir to add the script that adds the new header similar to the one you put in the description with the old header?

Dockerfile Outdated
EXPOSE 80

ENTRYPOINT ["sh", "/custom-start.sh", "sh", "/docker-entrypoint.sh"]
Copy link

Choose a reason for hiding this comment

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

I guess "/docker-entrypoint.sh" refers to the other PR. But why do we need both scripts /custom-start.sh and /docker-entrypoint.sh?

@edevosc2c
Copy link
Member

Please rebase against master :).

@tkohr
Copy link

tkohr commented Dec 11, 2023

I think master is not up-to-date/relevant in this repo. So just needs a rebase on georchestra-datafeeder.

@f-necas
Copy link
Author

f-necas commented Dec 11, 2023

I rebased it on georchestra-datafeeder to be more relevant. It was an old thing.

@tkohr if you can review it again :)

@f-necas f-necas requested a review from tkohr December 11, 2023 09:54
Copy link

@tkohr tkohr left a comment

Choose a reason for hiding this comment

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

Thanks @f-necas changes LGTM.

Just didn't understand why you added <link href="https://fonts.googleapis.comicon?family=Material+Icons|Material+Icons+Outlined" rel="stylesheet"/> to the index.html in your previous commit, but not here anymore.

@f-necas f-necas merged commit 9eb272d into georchestra-datafeeder Dec 11, 2023
@f-necas f-necas deleted the new-header branch January 29, 2024 14:27
f-necas added a commit that referenced this pull request Jun 26, 2024
relying on nx cache for building docker image

disable nx-cloud

feat: include georchestra header

df: support ENV for settings

df: set base href to ./

chore: build docker image

datafeeder - Adding a gh action to build / publish a docker image

gh-action - fixing georchestra version (docker tag)

chore: lint

chore: add thesaurus url to settings

base-ref=/impot/'

nginx - redirect onto the index.html when a UUID is present in the URL

Used the following in order to write the rule:

https://github.com/georchestra/geonetwork-ui/blob/georchestra/apps/datafeeder/src/app/app-routing.module.ts

Note:

* Not completely tested at runtime, compilation / docker image OK,
  the rewrite rule has been tested in a running docker image though.
* There is still probably some code missing JS-side to manage these
  query string after the '#' char, as reloading the page after having
  uploaded a dataset does not reload the app at the exact same place.

Concerns: This might break the usage of the docker image outside of the
official georchestra docker composition, as the redirection needs the
X-forwarded-* headers in order to work properly.

github-actions - deactivate gh-pages related actions

nginx - no redirect, just serve the index.html

npm run format:fix

fixing serving virtual routes for the JS app

somehow nginx will return a document with an "application/octet-stream"
content-type, because it probably can't figure out the type by itself.

Forcing "text/html" to be returned, so that the page is not "downloaded"
by the browser, but interpreted as a web page.

Tests: hacking the nginx configuration using the georchestra docker
composition. I was able to reload the page at the expected step once a
dataset has been uploaded.

chore: set proxy config for DF in georchestra

chore: use node 14 in docker image

change branch name in gh action

change branch name from datafeeder to georchestra-datafeeder

build(fix): fix dockerignore for datafeeder build

build(fix): raise build job timeout and formating

fixing lang calculation

some browsers seem to use the "xy-ZT" format for locates, which
obviously won't work as lookup key, as a two-char code is expected.

tests: runtime tested

implement & fix gh action to produce a docker image for the df frontend

See georchestra/georchestra#3659

Also fixes the existing action, as the branch has been renamed.

fix(nx): change defaultBase to georchestra-datafeeder branch

chore: add more to docker ignore

feat(docker): run postinstall scripts manually

This is required for Nx to build its dependency graph, and then to resolve
the result of `createGlobPatternsForDependencies` in the tailwind config

feat(header): make df header iframe configurable moving it from index.html to app.component

fix: formatting

Implement custom scripts (#7)

* feat: implement custom script

* change entrypoint name + fix entrypoint command

* let execution logic in upstream /docker-entrypoint.sh

---------

Co-authored-by: Emilien Devos <[email protected]>
georchestra: re-add icons

removal github actions workflow

see georchestra/georchestra#4105

Merge pull request #8 from georchestra/new-header
f-necas added a commit that referenced this pull request Jul 16, 2024
relying on nx cache for building docker image

disable nx-cloud

feat: include georchestra header

df: support ENV for settings

df: set base href to ./

chore: build docker image

datafeeder - Adding a gh action to build / publish a docker image

gh-action - fixing georchestra version (docker tag)

chore: lint

chore: add thesaurus url to settings

base-ref=/impot/'

nginx - redirect onto the index.html when a UUID is present in the URL

Used the following in order to write the rule:

https://github.com/georchestra/geonetwork-ui/blob/georchestra/apps/datafeeder/src/app/app-routing.module.ts

Note:

* Not completely tested at runtime, compilation / docker image OK,
  the rewrite rule has been tested in a running docker image though.
* There is still probably some code missing JS-side to manage these
  query string after the '#' char, as reloading the page after having
  uploaded a dataset does not reload the app at the exact same place.

Concerns: This might break the usage of the docker image outside of the
official georchestra docker composition, as the redirection needs the
X-forwarded-* headers in order to work properly.

github-actions - deactivate gh-pages related actions

nginx - no redirect, just serve the index.html

npm run format:fix

fixing serving virtual routes for the JS app

somehow nginx will return a document with an "application/octet-stream"
content-type, because it probably can't figure out the type by itself.

Forcing "text/html" to be returned, so that the page is not "downloaded"
by the browser, but interpreted as a web page.

Tests: hacking the nginx configuration using the georchestra docker
composition. I was able to reload the page at the expected step once a
dataset has been uploaded.

chore: set proxy config for DF in georchestra

chore: use node 14 in docker image

change branch name in gh action

change branch name from datafeeder to georchestra-datafeeder

build(fix): fix dockerignore for datafeeder build

build(fix): raise build job timeout and formating

fixing lang calculation

some browsers seem to use the "xy-ZT" format for locates, which
obviously won't work as lookup key, as a two-char code is expected.

tests: runtime tested

implement & fix gh action to produce a docker image for the df frontend

See georchestra/georchestra#3659

Also fixes the existing action, as the branch has been renamed.

fix(nx): change defaultBase to georchestra-datafeeder branch

chore: add more to docker ignore

feat(docker): run postinstall scripts manually

This is required for Nx to build its dependency graph, and then to resolve
the result of `createGlobPatternsForDependencies` in the tailwind config

feat(header): make df header iframe configurable moving it from index.html to app.component

fix: formatting

Implement custom scripts (#7)

* feat: implement custom script

* change entrypoint name + fix entrypoint command

* let execution logic in upstream /docker-entrypoint.sh

---------

Co-authored-by: Emilien Devos <[email protected]>
georchestra: re-add icons

removal github actions workflow

see georchestra/georchestra#4105

Merge pull request #8 from georchestra/new-header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants