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

4216 media editor and maps #4278

Merged
merged 5 commits into from
Oct 4, 2019
Merged

4216 media editor and maps #4278

merged 5 commits into from
Oct 4, 2019

Conversation

MV88
Copy link
Contributor

@MV88 MV88 commented Oct 2, 2019

Description

This pr add possibility to add maps to the background via the media editor
some additions:

  • media editor has a service menu to select which source needs to be used
  • media editor has a map preview
  • video tab temporary hidden
  • there is a first implementation of map component

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes, and I documented them in migration notes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@MV88 MV88 added the GeoStory label Oct 2, 2019
@MV88 MV88 requested a review from offtherailz October 2, 2019 14:22
@MV88 MV88 self-assigned this Oct 2, 2019
@MV88 MV88 mentioned this pull request Oct 2, 2019
12 tasks
@coveralls
Copy link

coveralls commented Oct 2, 2019

Coverage Status

Coverage decreased (-0.1%) to 84.747% when pulling ed3502d on MV88:4216_rebased into 44b985d on geosolutions-it:master.

@@ -92,7 +95,7 @@ export const openMediaEditorForNewMedia = action$ =>
Observable.of(
update(
path,
{ resourceId: resource.id, type: "image" }, // TODO take type from mediaEditor state or from resource
{ resourceId: resource.id, type: currentMediaTypeSelector(store.getState()) }, // TODO take type from mediaEditor state or from resource
Copy link
Member

@offtherailz offtherailz Oct 4, 2019

Choose a reason for hiding this comment

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

Suggested change
{ resourceId: resource.id, type: currentMediaTypeSelector(store.getState()) }, // TODO take type from mediaEditor state or from resource
{ resourceId: resource.id, type: currentMediaTypeSelector(store.getState()) },

const resourceAlreadyPresent = head(resourcesSelector(state).filter(r => r.data && (sourceId !== SourceTypes.GEOSTORY && r.data.id || r.id) === resource.id));

let resourceId = resource.id;
if (!resourceAlreadyPresent && resource.type === MediaTypes.MAP && sourceId !== SourceTypes.GEOSTORY) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't need to be a map. Every content from local source should not be added twice

Suggested change
if (!resourceAlreadyPresent && resource.type === MediaTypes.MAP && sourceId !== SourceTypes.GEOSTORY) {
if (!resourceAlreadyPresent && sourceId !== SourceTypes.GEOSTORY) {


let resourceId = resource.id;
if (!resourceAlreadyPresent && resource.type === MediaTypes.MAP && sourceId !== SourceTypes.GEOSTORY) {
// if using a geostore map that is not present in the story => add it
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// if using a geostore map that is not present in the story => add it
// if the resource is new, add it to the story resources list

export const lists = {
StoryTypes: values(StoryTypes),
SectionTypes: values(SectionTypes),
MediaTypes: values(MediaTypes),
Modes: values(Modes)
};

export const defaultLayerMapPreview = {
Copy link
Member

Choose a reason for hiding this comment

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

same here, in mediaeditor utils

Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

  • please separate the utilities that are proper of the media editor from GeoStory
    MediaEditor is a separate thing, should not know geostory, only the resource type.
  • see my comments
  • Solve issues with map catalog in widgets

@MV88 MV88 merged commit c04c5e2 into geosolutions-it:master Oct 4, 2019
@allyoucanmap allyoucanmap mentioned this pull request Aug 9, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants