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

GeocoderWidget support for LDS-based gecoding for CARTO 3 #387

Merged
merged 5 commits into from
Apr 28, 2022

Conversation

zbigg
Copy link
Contributor

@zbigg zbigg commented Apr 28, 2022

Description

Shortcut: https://app.shortcut.com/cartoteam/story/225672/address-search-in-builder

Enchance GeocoderWidget so it can geocode using LDS and can be embedded in builder

  • for V3 credentials, use LDS service
  • provide useGeocoderWidgetController hook that controls GeocoderWidget if UI is to be customized

Full backward compatibility for V2 credentials.

Type of change

  • Feature

Basic checklist

  • Good PR name
  • Shortcut link
  • Changelog entry
  • Just one issue per PR
  • GitHub labels
  • Proper status & reviewers
  • Tests
  • Documentation

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #225672: Address search in Builder.

@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch from 45a4286 to 54697b3 Compare April 28, 2022 07:08
@coveralls
Copy link
Collaborator

coveralls commented Apr 28, 2022

Pull Request Test Coverage Report for Build 2240320265

  • 38 of 46 (82.61%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 75.04%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/react-api/src/api/SQL.js 12 15 80.0%
packages/react-api/src/api/lds.js 16 21 76.19%
Totals Coverage Status
Change from base Build 2234346074: -0.07%
Covered Lines: 1517
Relevant Lines: 1904

💛 - Coveralls

@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch from 54697b3 to 49752bf Compare April 28, 2022 07:12
@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch from 49752bf to 8c08890 Compare April 28, 2022 07:16
@zbigg zbigg marked this pull request as ready for review April 28, 2022 07:18
@VictorVelarde VictorVelarde self-assigned this Apr 28, 2022
@VictorVelarde VictorVelarde requested a review from Josmorsot April 28, 2022 08:54
@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch from 85f8a3f to cb2c466 Compare April 28, 2022 09:59
@zbigg zbigg requested review from Clebal and borja-munoz April 28, 2022 09:59
Copy link
Contributor

@Clebal Clebal left a comment

Choose a reason for hiding this comment

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

Great work @zbigg!

Copy link
Contributor

@borja-munoz borja-munoz left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

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

LGTM, nice job

@VictorVelarde VictorVelarde changed the title GeocoderWidget support for LDS-based gecoding. GeocoderWidget support for LDS-based gecoding for CARTO 3 Apr 28, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch from 952b613 to cdf92ba Compare April 28, 2022 14:24
@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch 2 times, most recently from f189b52 to 1471c41 Compare April 28, 2022 15:31
@zbigg zbigg force-pushed the feature/sc-225672/address-search-in-builder branch from 1471c41 to 771a508 Compare April 28, 2022 15:32
@zbigg zbigg merged commit b429a9a into master Apr 28, 2022
@zbigg zbigg deleted the feature/sc-225672/address-search-in-builder branch April 28, 2022 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants