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

[SSDK-399] Add Discover.Options(country, proximity, origin) #167

Merged

Conversation

aokj4ck
Copy link
Contributor

@aokj4ck aokj4ck commented Jan 31, 2024

Description

Ticket: SSDK-399

  • Add Country.default: Self? to auto-detect the current device region and provide a value if found
  • Add proximity and origin CLLocationCoordinate2D fields to Discover.Option and forward these to SearchOption parameters
  • This fixes an issue when using search-along-route to query category results by providing a country, proximity, and origin parameters to Discover.Options.

Checklist

  • Update CHANGELOG

…, origin) params - SSDK-399

- Add Country.default: Self? to auto-detect the current device region and provide a value if found
- Add proximity and origin CLLocationCoordinate2D fields to Discover.Option and forward these to SearchOption parameters
…399-add-country-proximity-origin-to-discover-options

Conflicts:
	CHANGELOG.md
…399-add-country-proximity-origin-to-discover-options

Conflicts:
	CHANGELOG.md
…399-add-country-proximity-origin-to-discover-options

Conflicts:
	CHANGELOG.md
	MapboxSearch.xcodeproj/project.pbxproj
	Tests/MapboxSearchUITests/MockWebServer/MockResponse.swift
	Tests/MapboxSearchUITests/MockWebServer/MockWebServer.swift
@aokj4ck aokj4ck marked this pull request as ready for review February 8, 2024 17:16
@aokj4ck aokj4ck requested review from a team as code owners February 8, 2024 17:16
aokj4ck and others added 3 commits February 8, 2024 12:21
…399-add-country-proximity-origin-to-discover-options

Conflicts:
	CHANGELOG.md
	MapboxSearch.xcodeproj/project.pbxproj
} else {
let regionComponents = Locale.current.identifier.components(separatedBy: "_")
if regionComponents.count >= 2 {
let countryIdentifier = regionComponents[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @aokj4ck is countryIdenitifier only the last component in regionComponents or it is just the second item all the time?

just smal nitpick from my side: is it possible to use regionComponents?.last or regionComponents?.first to leave all these checks on compiler shoulders? Instead to access item via index in array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question, it is the second item all the time and there may be more items.

For example if you:

  1. Add an iOS 15 simulator (oldest simulator available in Xcode 14 and that uses this API)
  2. Change the Demo app > Edit Scheme > Run > Options > App Language and App Region
  3. Set a breakpoint on this line
  4. Build and run
  5. Go to Discover tab and tap "Search in this area"

You will find that there are scenarios where the locale has more components such as Language = Chinese (Hong Kong) and Region = Hong Kong will return the locale "en_HK_HK". There are various other examples and accessing the second items in the regionComponents is safest to cover all scenarios.

Screenshot 2024-02-14 at 10 16 17 Screenshot 2024-02-14 at 10 18 44

chizhavko
chizhavko previously approved these changes Feb 14, 2024
Copy link
Contributor

@chizhavko chizhavko left a comment

Choose a reason for hiding this comment

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

LGTM

@aokj4ck aokj4ck mentioned this pull request Feb 14, 2024
4 tasks
@aokj4ck aokj4ck merged commit 70b8a50 into main Feb 15, 2024
4 checks passed
@aokj4ck aokj4ck deleted the SSDK-399-add-country-proximity-origin-to-discover-options branch February 15, 2024 14:18
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.

3 participants