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

Integrate layerList to store and add breadcrumbs #80

Merged
merged 5 commits into from
Nov 10, 2022

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Nov 2, 2022

Signed-off-by: Junqiu Lei [email protected]

Description

  • Integrate map layers to store
  • Add saved map to route in the format of app/maps-dashboards/{id}
  • Add breadcrumbs to listing page, create and saved map page

Issues Resolved

#68

Demo

map_demo_integrate_layers_to_store.mov

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.

@junqiu-lei junqiu-lei self-assigned this Nov 2, 2022
@junqiu-lei junqiu-lei requested a review from a team November 2, 2022 17:26
@VijayanB
Copy link
Member

VijayanB commented Nov 2, 2022

Shall we update breadcrumb immediately with name once it is saved for first time?

@junqiu-lei
Copy link
Member Author

junqiu-lei commented Nov 2, 2022

Shall we update breadcrumb immediately with name once it is saved for first time?

Makes sense to immediately update breadcrumb once saved. Updated the code, you could check the updated demo as well.

@junqiu-lei junqiu-lei requested a review from VijayanB November 2, 2022 20:17
Signed-off-by: Junqiu Lei <[email protected]>
@junqiu-lei
Copy link
Member Author

@ashwin-pc Thanks your helpful feedback here and offline discussion! I updated the PR to make fetch saved object at top level component, and for the layers hook, I found most of components need use it and it doesn't have too much levels of component, I prefer to remain the existing hooks to use.

Signed-off-by: Junqiu Lei <[email protected]>
VijayanB
VijayanB previously approved these changes Nov 8, 2022
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

The updates look nice! The code is much more readable. A few additional comments, some suggestions and a few questions. No real blockers. Just want to see your responses to my comments before approving.

Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

LGTM

@junqiu-lei junqiu-lei merged commit 1d768bd into opensearch-project:feature/new-maps Nov 10, 2022
@junqiu-lei junqiu-lei deleted the map_route branch November 10, 2022 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants