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.

Sign up for GitHub

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

Jump to bottom

Identify layer features Sample #140

Merged
merged 18 commits into from
Nov 9, 2023
Merged

Identify layer features Sample #140

merged 18 commits into from
Nov 9, 2023

Conversation

prupani-7
Copy link

Description

PR to add a new Kotlin sample "Identify layer features" in Search and Query category.

Links and Data

Sample Epic: runtime/kotlin/issues/2934

What To Review

  • Review the code to make sure it is easy to follow like other samples on Android
  • README.md and README.metadata.json files

How to Test

Run the sample on the sample viewer or the repo.

@prupani-7 prupani-7 changed the base branch from main to v.next October 24, 2023 18:15
@prupani-7 prupani-7 changed the base branch from v.next to main October 24, 2023 18:16
@prupani-7 prupani-7 changed the base branch from main to v.next October 24, 2023 18:16
@shubham7109 shubham7109 added the New sample New Kotlin sample using ArcGIS Maps SDK label Oct 27, 2023
@shubham7109 shubham7109 self-requested a review October 27, 2023 19:39
shubham7109
shubham7109 requested changes Oct 27, 2023
View reviewed changes
Copy link
Collaborator

@shubham7109 shubham7109 left a comment

Choose a reason for hiding this comment

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

Nice sample @prupani-7! Couple comments here:

identify-layer-features/README.md Outdated Show resolved Hide resolved
identify-layer-features/README.md Outdated Show resolved Hide resolved
.../src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/ComposeMapView.kt Outdated Show resolved Hide resolved
.../src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/ComposeMapView.kt Outdated Show resolved Hide resolved
...eatures/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/screens/MainScreen.kt Outdated Show resolved Hide resolved
...es/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/MapViewModel.kt Outdated Show resolved Hide resolved
...es/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/MapViewModel.kt Outdated Show resolved Hide resolved
...es/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/MapViewModel.kt Outdated Show resolved Hide resolved
...es/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/MapViewModel.kt Outdated Show resolved Hide resolved
...es/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/MapViewModel.kt Outdated Show resolved Hide resolved
prupani-7 and others added 12 commits October 30, 2023 12:55
@prupani-7 @shubham7109
Update identify-layer-features/README.md
1f52322 
Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/README.md
cf3ee44 
Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
64cc9e4 
…le/identifylayerfeatures/components/ComposeMapView.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
476700c 
…le/identifylayerfeatures/components/ComposeMapView.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
526055c 
…le/identifylayerfeatures/screens/MainScreen.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
5d2f634 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
9d0de97 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
b98db36 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
e323695 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
1e67c8b 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
48ce390 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
@prupani-7
feedback from Shubham
3e440ab
@prupani-7
Copy link
Author

@shubham7109 Thanks for the review suggestions! Applied. please re-review?

shubham7109
shubham7109 approved these changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@shubham7109 shubham7109 left a comment

Choose a reason for hiding this comment

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

Looks good to approve 👍

...es/src/main/java/com/esri/arcgismaps/sample/identifylayerfeatures/components/MapViewModel.kt Show resolved Hide resolved
@prupani-7 @shubham7109
Update identify-layer-features/src/main/java/com/esri/arcgismaps/samp…
b661d12 
…le/identifylayerfeatures/components/MapViewModel.kt

Co-authored-by: Shubham Sharma <shubhamsharma@esri.com>
rchintapalli
rchintapalli approved these changes Nov 8, 2023
View reviewed changes
Copy link
Collaborator

@rchintapalli rchintapalli left a comment

Choose a reason for hiding this comment

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

Not sure if it was part of original sample design or not, but selecting/highlighting the identified features also gives better visual experience.

Looks good to me otherwise

@prupani-7
Copy link
Author

@rchintapalli sample design doesn't do feature selecting/highlighting. So leaving this as is. Thanks for your review.

@prupani-7 prupani-7 merged commit ae02290 into v.next Nov 9, 2023
1 check passed
@prupani-7 prupani-7 deleted the pri/identifylayerfeatures branch November 9, 2023 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rchintapalli rchintapalli rchintapalli approved these changes

@shubham7109 shubham7109 shubham7109 approved these changes

Assignees

@prupani-7 prupani-7

Labels
New sample New Kotlin sample using ArcGIS Maps SDK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@prupani-7 @rchintapalli @shubham7109