Skip to content

Add dedicated sample pages for Compass, Locate, BasemapToggle, and ScaleBar widgets#14

Merged
TimPurdum merged 7 commits into
mainfrom
feature/11-widget-sample-pages
Apr 29, 2026
Merged

Add dedicated sample pages for Compass, Locate, BasemapToggle, and ScaleBar widgets#14
TimPurdum merged 7 commits into
mainfrom
feature/11-widget-sample-pages

Conversation

@magmoe
Copy link
Copy Markdown
Contributor

@magmoe magmoe commented Apr 14, 2026

Summary

  • 4 new Core widget sample pages, each showcasing a widget that previously only appeared in the Widgets overview
  • Compass Widget (/compass-widget): rotation buttons with CompassWidget auto-reset
  • Locate Widget (/locate-widget): geolocation with configurable zoom scale
  • Basemap Toggle Widget (/basemap-toggle-widget): switchable basemap style pairs
  • Scale Bar Widget (/scale-bar-widget): unit and style toggle controls
  • Nav menu entries for all 4 new pages
  • Fix UniqueValueRenderers build error

…aleBar widgets

New pages:
- /compass-widget: Map rotation controls with CompassWidget reset
- /locate-widget: Geolocation with configurable zoom scale
- /basemap-toggle-widget: Switchable basemap style pairs
- /scale-bar-widget: Unit (Imperial/Metric/Dual) and style (Ruler/Line) toggles

Also adds nav menu entries and fixes UniqueValueRenderers build error.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

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

This PR expands the Core sample app by adding dedicated pages for several widgets (Compass, Locate, BasemapToggle, ScaleBar) that were previously only demonstrated in the Widgets overview, and includes a small fix to resolve a build error in the UniqueValueRenderers sample.

Changes:

  • Added 4 new widget-specific sample pages with simple configuration controls.
  • Updated the nav menu to include routes for the new pages.
  • Adjusted UniqueValueRenderers label formatting logic to avoid the removed/invalid ToUpperFirstChar() usage.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Shared/NavMenu.razor.cs Adds nav links for the four new widget sample pages.
samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/UniqueValueRenderers.razor.cs Replaces ToUpperFirstChar() usage with inline formatting to fix build.
samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/CompassWidgetPage.razor New Compass widget page with rotation controls.
samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/LocateWidgetPage.razor New Locate widget page with selectable zoom scale.
samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/BasemapToggleWidgetPage.razor New BasemapToggle widget page with selectable basemap pairs.
samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/ScaleBarWidgetPage.razor New ScaleBar widget page with unit/style controls.

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


private void OnScaleChanged(ChangeEventArgs e)
{
_scale = int.Parse(e.Value?.ToString() ?? "1500");

private void OnPairChanged(ChangeEventArgs e)
{
_pairIndex = int.Parse(e.Value?.ToString() ?? "0");
Comment thread samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/ScaleBarWidgetPage.razor Outdated
Comment on lines +54 to +60
_unit = Enum.Parse<ScaleUnit>(e.Value?.ToString() ?? "Imperial");
_widgetKey++;
}

private void OnStyleChanged(ChangeEventArgs e)
{
_style = Enum.Parse<ScaleBarWidgetStyle>(e.Value?.ToString() ?? "Ruler");
Comment on lines 62 to 64
private readonly UniqueValueRenderer _uniqueValueRenderer = new(uniqueValueInfos: roadTypes
.Select(r => new UniqueValueInfo(r.Key.ToUpperFirstChar().Replace("_", " "), r.Value, r.Key))
.Select(r => new UniqueValueInfo(string.Concat(r.Key[0].ToString().ToUpper(), r.Key.AsSpan(1)).Replace("_", " "), r.Value, r.Key))
.ToArray(),
<span style="margin-left: 1rem; opacity: 0.7;">Current: @(_rotation)&deg;</span>
</div>

<MapView @key="@_rotation" Longitude="-118.805" Latitude="34.027" Zoom="13" Rotation="@_rotation" Class="map-view">
Comment thread samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/LocateWidgetPage.razor Outdated
- Add value="@_pairIndex" to BasemapToggle select
- Add value="@_unit" and value="@_style" to ScaleBar selects
- Add aria-hidden="true" and focusable="false" to locate icon SVG

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Code Review — PR #14: Compass / Locate / BasemapToggle / ScaleBar widget samples

Overview

Four dedicated pages for widgets that were previously only demoed inside the big "Widgets" overview. Each page has dropdown/button controls to exercise the widget's props. Good additions. Also includes the UniqueValueRenderers build fix.

🔴 Architectural concern — @key on MapView

Every page here uses @key on MapView, bound to a state field that changes on user interaction:

<MapView @key="@_rotation"   Rotation="@_rotation" ...>   <!-- CompassWidgetPage -->
<MapView @key="@_widgetKey"  ...>                         <!-- LocateWidgetPage, ScaleBarWidgetPage -->
<MapView @key="@_pairIndex"  ...>                         <!-- BasemapToggleWidgetPage -->

This forces Blazor to dispose and recreate the entire MapView (and the underlying ArcGIS JS View, which is expensive) on every user action. A map reload on each button click for a rotation change is a bad demo experience — the map will visibly flash/reload.

The correct GeoBlazor pattern is parameter binding:

  • MapView.Rotation is a parameter and updates reactively — drop @key on the Compass page.
  • LocateWidget.Scale / ScaleBarWidget.Unit / ScaleBarWidget.Style should propagate through normal parameter binding. If they don't, that's a GeoBlazor bug worth filing in the main repo rather than working around here with @key. I'd check first:
    • Does await _locateWidget.SetScale(...) exist? (GeoBlazor often exposes imperative setters alongside params.)
    • Is there a known issue about widget-property reactivity?

For BasemapTogglePage where the Secondary basemap style changes, a full re-mount may genuinely be the only option today — but it's worth confirming there isn't a SetNextBasemapStyle or similar.

At minimum, if @key is truly required, add a comment explaining why, so this doesn't become the go-to pattern copied into other samples.

🟡 Smaller issues

  1. int.Parse / Enum.Parse on user input.

    _pairIndex = int.Parse(e.Value?.ToString() ?? "0");
    _unit = Enum.Parse<ScaleUnit>(e.Value?.ToString() ?? "Imperial");

    Throws on unexpected values. In practice a <select> never emits an unexpected value, but TryParse with a fallback is idiomatic Blazor. Or better — @bind directly to the enum / int and let Blazor do the conversion:

    <select @bind="_unit">
        @foreach (var u in Enum.GetValues<ScaleUnit>())
        {
            <option value="@u">@u</option>
        }
    </select>
  2. LocateWidget: Scale is typed int — confirm that matches GeoBlazor's parameter type (it may be double?). If double?, the parse + state could be simplified.

  3. Locate page instructions use inline SVG to show the button glyph — cute but fragile if the Esri icon set changes. A screenshot reference or textual description is more durable.

  4. Nav entries inserted mid-list between "widgets" and "basemaps" in NavMenu.razor.cs. When Add collapsible category submenus to navigation sidebar #13 merges, these will need Category: Categories.Widgets to slot into the new group structure. See my note on Add collapsible category submenus to navigation sidebar #13.

🟢 Nice touches

  • Each page links to both the ArcGIS JS sample and the API reference — consistent with the repo convention.
  • Current-rotation readout ("Current: 45°") on the Compass page is a nice feedback touch.
  • ScaleBarWidget supports both unit and style — good to exercise both dimensions.

Merge ordering

Conflicts with #12 on UniqueValueRenderers.razor.cs (identical fix) and with #13 on NavMenu.razor.cs. Suggest merge order: #12#14#13, with #13 responsible for categorizing the new pages from #12 and #14.


Generated by Claude Code

# Conflicts:
#	samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Pages/UniqueValueRenderers.razor.cs
</MapView>

@code {
private double _rotation = 45;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would start at 0, it was confusing as a user because the first button I tried was 45 and nothing happened.

<line x1="0" y1="8" x2="3" y2="8" stroke="currentColor" stroke-width="1.5"/>
<line x1="13" y1="8" x2="16" y2="8" stroke="currentColor" stroke-width="1.5"/>
</svg> button (top-left of the map) to fly to your location.
Change the zoom scale below, then click it again to see the difference in zoom level.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should add instructions about the "Allow" permissions dialog that will pop up in most browsers.


<div style="display: flex; flex-wrap: wrap; gap: 0.5rem 1.5rem; margin-bottom: 1rem;">
<div class="form-group">
<label>Zoom Scale:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This terminilogy is confusing, since ArcGIS/GeoBlazor have both Zoom and Scale properties. I would just use the word Scale since that is the property you are targeting, and remove the word zoom completely.

and visual style.
</p>

<div style="display: flex; flex-wrap: wrap; gap: 0.5rem 1.5rem; margin-bottom: 1rem;">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Avoid inline styles on all our samples. See the changes that I made on PR 12. For example, I would make this div class="form-group", and the other two class="spaced-row".

<option value="Metric">Metric (km/m)</option>
<option value="Dual">Dual (both)</option>
</select>
</label>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've moved to not wrapping with labels, it was breaking layout for me. Instead, use for:

<label for="unit-select">
    Unit:
</label>
<select id="unit-select" @onchange="...">
...
</select>

Also, I believe by removing the class and style this select will default to match the other pages, but you can check and confirm.

<a class="btn btn-secondary" target="_blank" href="https://developers.arcgis.com/javascript/latest/sample-code/widgets-basemaptoggle/">ArcGIS Sample</a>
<a class="btn btn-primary" target="_blank" href="https://developers.arcgis.com/javascript/latest/api-reference/esri-widgets-BasemapToggle.html">BasemapToggle API Reference</a>
</div>
<p class="instructions">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See my changes moving links-div links to PageLinks in the @code block. I would like you to update the other 4 samples in the same way.

<option value="2">Navigation / Imagery</option>
<option value="3">OSM Standard / Terrain</option>
</select>
</label>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see notes on ScaleBar page about styling

<button class="btn btn-secondary btn-small" @onclick="@(() => SetRotation(270))">270&deg;</button>
<button class="btn btn-accent btn-small" @onclick="@(() => SetRotation(0))">Reset North</button>
<span style="margin-left: 1rem; opacity: 0.7;">Current: @(_rotation)&deg;</span>
</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see notes on ScaleBar page about styling

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in this case, you can combine form-group and spaced-row to get a similar effect. It's fine to use the style block if you have to override the final result of those classes, but try those first.

<option value="100000">100,000 (Region)</option>
</select>
</label>
</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see ScaleBar page style notes

Refactor Compass, Locate, ScaleBar, and BasemapToggle pages:

- Migrate to PageLinks pattern (inherit SamplePage, drop links-div)
- Replace inline styles with form-group / spaced-row classes
- Use label for=/select id= pattern instead of wrapping selects
- Drop form-select class and inline widths
- Replace manual int.Parse / Enum.Parse handlers with @Bind

Drop @key on Compass, Locate, and ScaleBar so parameter changes
update reactively without re-mounting the MapView. BasemapToggle
keeps @key (with explanatory comment) since the primary basemap
swap inside <Map> isn't reactive today. Compass uses an imperative
SetRotation via @ref for smooth animation, plus an OnExtentChanged
hook so the readout follows on-map compass clicks.

Wording fix on Locate: drop "zoom" from user-facing text; the
property targeted is Scale.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@magmoe magmoe requested a review from TimPurdum April 28, 2026 21:15
# Conflicts:
#	samples/core/dymaptic.GeoBlazor.Core.Sample.Shared/Shared/NavMenu.razor.cs
@TimPurdum TimPurdum merged commit da15af4 into main Apr 29, 2026
2 checks passed
@TimPurdum TimPurdum deleted the feature/11-widget-sample-pages branch April 29, 2026 18:39
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