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

Add maps saved object for sample datasets #240

Merged

Conversation

naveentatikonda
Copy link
Member

@naveentatikonda naveentatikonda commented Feb 10, 2023

Description

This PR includes the changes which adds the dashboards-maps saved object to the existing sample dataset. When the user install a sample dataset then in the dashboards-maps panel, we can see a default saved map which will help maps users to explore maps with that dataset instead of creating a new map from scratch.

We have two videos below which shows the dashboards-maps panel before adding these changes and after adding these changes for flights sample dataset. Similarly, we can add saved objects for other sample datasets.

Before.adding.changes.mov
After.adding.changes.mov

Testing

Tested it to make sure the saved object consumes the latest changes in config(dataUrl and styleUrl):

  1. Firstly, without makes any changes to config values, such that it consumes default values from config.ts
  2. Passing it as a command line argument
yarn start --custom_import_map_dashboards.opensearchVectorTileDataUrl="...."
  1. Setting it's value in the config/opensearch_dashboards.yml

Issues Resolved

opensearch-project/OpenSearch-Dashboards#3244

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

naveentatikonda and others added 3 commits February 9, 2023 18:23
Signed-off-by: Naveen Tatikonda <[email protected]>
Signed-off-by: Naveen Tatikonda <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2023

Codecov Report

Merging #240 (55ee6d4) into main (ffb12e6) will increase coverage by 0.06%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
+ Coverage   91.60%   91.66%   +0.06%     
==========================================
  Files           9       10       +1     
  Lines         262      264       +2     
  Branches       34       34              
==========================================
+ Hits          240      242       +2     
  Misses         16       16              
  Partials        6        6              
Impacted Files Coverage Δ
...ponents/layer_control_panel/delete_layer_modal.tsx 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@naveentatikonda naveentatikonda changed the title Add maps saved object Add maps saved object for sample datasets Feb 10, 2023
server/plugin.ts Outdated Show resolved Hide resolved
description: 'Shows cancelled flights',
type: 'documents',
id: 'f3ae28ce-2494-4e50-ae31-4603cfcbfd7d',
zoomRange: [2, 22],
Copy link
Member

Choose a reason for hiding this comment

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

How about make the example of document layer to be visible when user initially open it at zoom 0, like [0, 5]? IMO, as a demo example, it should be visualization friendly when at first glance.

Copy link
Member

Choose a reason for hiding this comment

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

IMO, We should have use case why some layers can be invisible from beginning. User might not notice if everything is visible starting from 0.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, so that we can show layer is invisible after like zoom 5. I think the initially display on demo example is important for new users to show the main features. But yeah, any choice should be good.

Copy link
Member

Choose a reason for hiding this comment

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

Lets make it between [0 - 14] like junqiu suggested.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I will set the zoom range to [0-14] in the sample object of actual default map while checking it into the repo. As this is a dummy sample object just to validate the changes. For now, I'm leaving it as it is.

import { i18n } from '@osd/i18n';
import { ConfigSchema } from '../../../common/config';

const layerList = (config: ConfigSchema) => [
Copy link
Member

Choose a reason for hiding this comment

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

nit: can this be destructured?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried. But, not able to destructure it.

@VijayanB VijayanB added enhancement New feature or request and removed feature labels Feb 13, 2023
Signed-off-by: Naveen Tatikonda <[email protected]>
Copy link
Member

@VijayanB VijayanB left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks.

@naveentatikonda naveentatikonda merged commit 816ce79 into opensearch-project:main Feb 14, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 14, 2023
* Add Default Saved object for Maps

Signed-off-by: Naveen Tatikonda <[email protected]>

* Consume config and output result

Signed-off-by: Kawika Avilla <[email protected]>

* Refactor the code

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
Signed-off-by: Kawika Avilla <[email protected]>
Co-authored-by: Kawika Avilla <[email protected]>
(cherry picked from commit 816ce79)
naveentatikonda added a commit that referenced this pull request Feb 16, 2023
* Add Default Saved object for Maps

Signed-off-by: Naveen Tatikonda <[email protected]>

* Consume config and output result

Signed-off-by: Kawika Avilla <[email protected]>

* Refactor the code

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
Signed-off-by: Kawika Avilla <[email protected]>
Co-authored-by: Kawika Avilla <[email protected]>
(cherry picked from commit 816ce79)

Co-authored-by: Naveen Tatikonda <[email protected]>
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.

5 participants