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

Kevin 385 active pin #569

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Kevin 385 active pin #569

wants to merge 9 commits into from

Conversation

icycoldveins
Copy link
Collaborator

Pull Request

Change Summary

• Refactored map marker icon logic to differentiate between active and default states based on resource selection.

• Implemented dynamic icon switching using a conditional CASE structure for resource types such as Water, Foraging, Bathroom, and Food.

• Updated the logic to assign active icons when a resource is selected and default icons when unselected.

Change Reason

• This change improves user experience by providing clear visual cues for selected and unselected resources on the map.

• By distinguishing between active and default icons, users can more easily identify the selected resources and their types.

DancyyDev and others added 8 commits April 25, 2024 19:38
Conditionally render PinWaterActive for WATER resources at selected location
Updated icon source based on resource type and coordinates match
Used fallback to default marker icon if no match
Updated logic to display active icons for selected resources.
Ensured unselected resources use the default marker icons.
@gcardonag gcardonag linked an issue Feb 11, 2025 that may be closed by this pull request
@gcardonag
Copy link
Contributor

I'll give this PR a look shortly 👀
We may need to address some merge conflicts from the recent merge of a larger PR

@gcardonag gcardonag requested a review from RNR1 February 17, 2025 02:46
Copy link
Contributor

@gcardonag gcardonag left a comment

Choose a reason for hiding this comment

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

I just addressed the merge conflicts to get the updated code working with the larger changes. Since an eslint check did not like the use of nested ternaries (https://eslint.org/docs/latest/rules/no-nested-ternary) when setting the active icon URLs, I made a small adjustment to try and improve readability.

However, I think we can simplify things further by addressing the note I left for how the Active Icons are currently produced

Comment on lines +277 to +285
switch (resource.resource_type) {
case 'WATER':
return PinWaterActive();
case 'FOOD':
return PinFoodActive();
case 'FORAGE':
return PinForagingActive();
case 'BATHROOM':
return PinBathroomActive();
Copy link
Contributor

Choose a reason for hiding this comment

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

@icycoldveins , could you consolidate the Pin*Active files into a single file, similar to PhlaskMarkerIconV2.js and how it handles the resource type as an input parameter? That would help with consistency in this section

@icycoldveins
Copy link
Collaborator Author

icycoldveins commented Feb 17, 2025 via email

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.

Selected Tap should be marked by Active/Inverted Pin
3 participants