-
Notifications
You must be signed in to change notification settings - Fork 45
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
front: Initialize the new version of the Map. #10280
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10280 +/- ##
==========================================
+ Coverage 81.53% 81.59% +0.06%
==========================================
Files 1059 1060 +1
Lines 104459 104928 +469
Branches 722 722
==========================================
+ Hits 85166 85612 +446
- Misses 19252 19275 +23
Partials 41 41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
89dff49
to
fbc8a55
Compare
@@ -162,7 +162,7 @@ class STDCMPage { | |||
this.notificationHeader = page.locator('#notification'); | |||
this.debugButton = page.getByTestId('stdcm-debug-button'); | |||
this.helpButton = page.getByTestId('stdcm-help-button'); | |||
this.mapContainer = page.locator('#map-container'); | |||
this.mapContainer = page.locator('stdcm-map-config'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
page.locator('#stdcm-map-config');
just to be sure, is the name |
Oh no, the name will also change once the refactor is complete |
withSearchButton?: boolean; | ||
withToggleLayersButton?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with
is usually used for HOC components I guess, it can be a little bit confusing, should'nt we use is
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just followed the naming convention used by other props that were already present like withMapKeyButton
or withInfraButton
. Would you be okay if we address this in the next step of the refactor?
cd17d0a
to
343322d
Compare
This commit initializes the new version of the Map component. The Map is now decoupled from the store. Specifically, the zoom button callbacks are detached from the store, while the other buttons are temporarily hidden. Signed-off-by: nncluzu <[email protected]>
343322d
to
b225664
Compare
closes #9776
This commit initializes the refacto of the Map component. The Map is now decoupled from the store. Specifically:
This change allows multiple Map instances to be rendered on the same page without conflicts.