-
Notifications
You must be signed in to change notification settings - Fork 3
#22 Attribution Not Displaying When Selecting a Feature #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
base: main
Are you sure you want to change the base?
Conversation
…ng the proper attribution
- name: Deploy to GitHub Pages | ||
id: deployment | ||
uses: actions/deploy-pages@v4 |
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 don't use github pages (yet?), deployment is directly from main. This means if we push to main via CI, we need to ensure that no infinite CI loop is created.
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.
Though I don't think we're in any way opposed to using the deploy-pages action. So if it's a better route then you could just update it so the whole site gets deployed by actions instead of just pointing at main.
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.
If I am interpreting this correctly by looking into the actions for the repo. We are using github pages, we're just having to manually run the build step on the correct folder and then do a push to main.
The default actions of pointing to main is just looking at the index.html file and building that, which isn't where any of the recent changes have been occurring. My changes should run the build step on the correct folder, /map
and upload the artifact to deploy to the github pages.
It would be similar to what is currently in place with a step to build the correct folder before deploying.
map-src/functions/MapUtils.js
Outdated
attribution.innerHTML = `Sentinel-2 cloudless - <a href="https://s2maps.eu" target="_blank">https://s2maps.eu</a> by EOX IT Services GmbH (Contains modified Copernicus Sentinel data ${selectedYear})`; | ||
attribution.style.display = 'block'; |
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.
That looks a bit hacky. Shouldn't that be handled by OL's attribution options for the layers? There's also a collapsed option in the Attribute layer to show the attributions directly.
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.
updated this to use the OL attribution prop.
Along with this PR I added some tests as well as took a stab at adding some Github Actions to help with deploying the changes that are made along the way and hopefully bypassing the manual process and testing PR branches to ensure the core functionality remains in place.