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.

Sign up for GitHub

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

Jump to bottom

add imagery credits for change detection project type #224 #564

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

add imagery credits for change detection project type #224 #564

wants to merge 10 commits into from

Conversation

Hagellach37
Copy link
Member

@Hagellach37 Hagellach37 commented Aug 18, 2021 •
edited

this adds the imagery credits to the change detection project mapping view. I'm not sure what's the best placement of the credit label.

Maybe we could also think about moving this code into a new component?

What do you think @laurentS ?

image

@Hagellach37
Copy link
Member Author

Hey @laurentS,

the ios build failed and it says: Your certificate 'MD6M8684YW.cer' is not valid, please check end date and renew it if necessary.

What does this mean?

@laurentS laurentS mentioned this pull request Aug 19, 2021
Closed
@laurentS
Copy link
Member

the ios build failed and it says: Your certificate 'MD6M8684YW.cer' is not valid, please check end date and renew it if necessary.

What does this mean?

Moved this to #565

@laurentS
Copy link
Member

this adds the imagery credits to the change detection project mapping view. I'm not sure what's the best placement of the credit label.

Maybe we could also think about moving this code into a new component?

What do you think @laurentS ?

Does the credit text have to be on this screen? I feel like it adds clutter. Could we have it on a separate info box, like with the main projects? Accessed via the "i" icon top right, maybe?

laurentS
laurentS previously approved these changes Aug 21, 2021
View reviewed changes
Copy link
Member

@laurentS laurentS left a comment

Choose a reason for hiding this comment

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

A couple of questions, but otherwise looks good to me :)

src/shared/views/ChangeDetection/Body.js Outdated Show resolved Hide resolved
src/shared/views/ChangeDetection/InstructionsScreen.js Outdated Show resolved Hide resolved
src/shared/views/ChangeDetection/index.js Outdated Show resolved Hide resolved
@Hagellach37
Copy link
Member Author

alright, thanks for the feedback. I adjusted the code based on your suggestions. :)

@Hagellach37
Merge pull request #595 from mapswipe/dev
b2d8d39 
Merge dev in feature branch
@Hagellach37
Copy link
Member Author

Hey @laurentS ,
I think that we've never merged this PR because we had some issues with the ios builds. But now these should be working and maybe you could give this one a last check.

Then we merge it into dev?

@Hagellach37 Hagellach37 linked an issue Nov 29, 2021 that may be closed by this pull request
display imagery credentials in change detection project #563
Closed
@Hagellach37 Hagellach37 added this to the 2.1.6 milestone Nov 29, 2021
Base automatically changed from dev to master November 30, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurentS laurentS laurentS left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.1.6
Development

Successfully merging this pull request may close these issues.

display imagery credentials in change detection project
2 participants
@Hagellach37 @laurentS