Skip to content

[REFACTORING] Indoor Poi Marker Factory#218

Merged
CanadianMaple-Syrup merged 12 commits into
masterfrom
indoor-poi-marker-refactor
Apr 6, 2026
Merged

[REFACTORING] Indoor Poi Marker Factory#218
CanadianMaple-Syrup merged 12 commits into
masterfrom
indoor-poi-marker-refactor

Conversation

@CanadianMaple-Syrup
Copy link
Copy Markdown
Collaborator

@CanadianMaple-Syrup CanadianMaple-Syrup commented Apr 5, 2026

Summary

Attempts to replace the current marker rendering with a simple factory method implementation

Took inspiration from a mix of textbook factory method and this article

Related Issues

Closes: N/A

Changes Made

  • Reorganized some component files to separate outdoor map components from indoor ones
  • Separated the indoor POI markers component from the indoor map
  • Created a factory for indoor markers for extensibility and maintainability purposes

Testing

  • Manual testing performed (Ran the application on my emulator to ensure that it still worked the same and that I didn't break any features)
  • Unit tests added or updated (if applicable)
  • All tests pass locally / in CI

Checklist (Author & Reviewer)

  • Code follows project conventions
  • The change was run locally and works as expected
  • Acceptance criteria met
  • No breaking changes

Copilot AI review requested due to automatic review settings April 5, 2026 23:37
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 5, 2026

Codecov Report

❌ Patch coverage is 94.73684% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
components/map/indoor-map/building-floor.tsx 92.15% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the indoor map UI by moving indoor-specific components under components/map/indoor-map/ and introducing a factory-based approach for rendering indoor POI markers to make marker rendering more extensible.

Changes:

  • Added a PoiFilters type to centralize indoor POI filter state typing.
  • Split indoor map UI into dedicated components (room fields, settings, navigation controls, floor selection).
  • Replaced inline POI marker rendering with a DefaultMarkerFactory + DefaultPoiMarker implementation.

Reviewed changes

Copilot reviewed 11 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
types/mapTypes.ts Adds shared PoiFilters type for indoor POI filtering.
components/map/indoor-map/indoor-room-fields.tsx New indoor route start/end room inputs with suggestion UI.
components/map/indoor-map/indoor-poi/default-poi-marker.tsx Extracted reusable SVG marker (icon + optional highlight).
components/map/indoor-map/indoor-poi/default-marker-factory.tsx Introduces factory to create POI markers by checkpoint type/metadata.
components/map/indoor-map/indoor-navigation-controls.tsx New component for step/floor navigation controls UI.
components/map/indoor-map/indoor-map-settings.tsx New settings UI for wheelchair routing + POI highlighting toggles.
components/map/indoor-map/floor-selection-menu.tsx New dropdown UI for selecting the active floor.
components/map/indoor-map/building-floor.tsx Indoor floor renderer updated to use the marker factory.
components/map/building-floor.tsx Removes old indoor floor renderer implementation from the prior location.
app/(tabs)/(map)/[buildingCode].tsx Updates imports to new indoor-map component locations.
__tests__/* Updates import/mocking paths to match the new component locations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread components/map/indoor-map/indoor-poi/default-marker-factory.tsx Outdated
Comment thread components/map/indoor-map/building-floor.tsx Outdated
Copy link
Copy Markdown
Collaborator

@YehJordan YehJordan left a comment

Choose a reason for hiding this comment

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

I think this is a good refactoring, and it is a good improvement from the previous code. However, I don't think we can call this a textbook Gang of Four factory design pattern since there are no usage of subclasses to decide the POI.

But the way I see it, its more like a typescript version of a factory pattern.

All previous functionalities work like before, and the indoor POI marker features look like they function the same as well. Good work 👍

@CanadianMaple-Syrup
Copy link
Copy Markdown
Collaborator Author

I think this is a good refactoring, and it is a good improvement from the previous code. However, I don't think we can call this a textbook Gang of Four factory design pattern since there are no usage of subclasses to decide the POI.

But the way I see it, its more like a typescript version of a factory pattern.

Yeah I agree it isn't quite the textbook version of it because every component is a "subclass" of the ReactElement so we can't quite use the subclassing aspect of the "product" side of things, but we do have a generic factory type, so its like having half of the textbook I guess?

Strange4
Strange4 previously approved these changes Apr 6, 2026
Copy link
Copy Markdown
Collaborator

@Strange4 Strange4 left a comment

Choose a reason for hiding this comment

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

Looks exactly like when we discussed in the thread. I think that this is a good improvement.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 6, 2026

@CanadianMaple-Syrup CanadianMaple-Syrup merged commit da25a17 into master Apr 6, 2026
4 checks passed
@CanadianMaple-Syrup CanadianMaple-Syrup deleted the indoor-poi-marker-refactor branch April 6, 2026 03:02
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.

4 participants