Skip to content

Split label view data#1313

Open
StempunkDev wants to merge 68 commits intoAzgaar:masterfrom
StempunkDev:feature/split-label-view-data
Open

Split label view data#1313
StempunkDev wants to merge 68 commits intoAzgaar:masterfrom
StempunkDev:feature/split-label-view-data

Conversation

@StempunkDev
Copy link
Copy Markdown
Contributor

@StempunkDev StempunkDev commented Feb 11, 2026

Description

This PR aims to split the view and data of the state and burg labels. What will be missing are the province labels. The goal is to get labels working as data as much as possible for states and burgs.

Type of change

  • Refactoring / style
  • Documentation update / chore

Versioning

  • Version is updated
  • Changed files hash is updated

@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 11, 2026

Deploy Preview for afmg ready!

Name Link
🔨 Latest commit 919a766
🔍 Latest deploy log https://app.netlify.com/projects/afmg/deploys/69fe63cd286f010008a7bdc0
😎 Deploy Preview https://deploy-preview-1313--afmg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

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 label handling to move state and burg label information into a data layer (pack.labels) while keeping rendering in SVG renderers, aiming to make labels more data-driven and reusable across the app.

Changes:

  • Added a Labels module to generate/store/update state and burg label data in pack.labels
  • Extracted state-label raycast logic into a reusable src/utils/label-raycast.ts utility
  • Updated state/burg label renderers and map generation to use/populate the new label data

Reviewed changes

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

Show a summary per file
File Description
src/utils/label-raycast.ts New shared raycast + scoring utilities for state label path selection
src/types/PackedGraph.ts Extends PackedGraph with labels: LabelData[]
src/renderers/draw-state-labels.ts Renders state labels from Labels data and updates label data (text/font size) during fitting
src/renderers/draw-burg-labels.ts Renders burg labels from Labels data and syncs offsets back to label data
src/modules/labels.ts New global Labels module for label CRUD + generation from states/burgs
src/modules/index.ts Ensures Labels module is loaded/registered
public/main.js Calls Labels.generate() during map generation

Comment thread public/main.js
Comment thread src/modules/labels.ts Outdated
Comment thread src/modules/labels.ts Outdated
Comment thread src/modules/labels.ts Outdated
Comment thread src/renderers/draw-state-labels.ts Outdated
Comment thread src/utils/label-raycast.ts Outdated
Comment thread src/renderers/label-raycast.ts Outdated
@StempunkDev
Copy link
Copy Markdown
Contributor Author

@SheepFromHeaven do you intend on migrating any of the editor in the near future? Since I would need to edit the burgs-editor.js for feature completeness. I hope I can safe both of us some merge conflicts

@SheepFromHeaven
Copy link
Copy Markdown
Collaborator

No worries. I will then leave it out for now. There is still enough other things. Incl. Fixing bugs 😂

@StempunkDev
Copy link
Copy Markdown
Contributor Author

StempunkDev commented Feb 19, 2026

Update: Mostly done. Custom Labels Migration from Older Maps still need to be tested.
Things to still apply if needed:

  • Renderer Optimization by composing HTML changes
  • getNextId Optimization
  • Generalize getNextId when wished for

@StempunkDev StempunkDev marked this pull request as ready for review February 19, 2026 18:48
@StempunkDev StempunkDev changed the title [WIP] Split label view data Split label view data Feb 24, 2026
@StempunkDev
Copy link
Copy Markdown
Contributor Author

This should now be test ready. The migration of labels might not fully keep the format since it approximates the textPath. TextPaths that are changed later should apply correctly.

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

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

Comments suppressed due to low confidence (1)

src/renderers/draw-state-labels.ts:152

  • State label rendering ignores persisted label properties: startOffset is hard-coded to 50% (and letterSpacing isn’t applied), even though these values are stored/edited via Labels.update(...). This makes label editor changes non-persistent across redraws. Use labelData.startOffset / labelData.letterSpacing (with defaults) when setting textPath attributes.
      const textElement = textGroup
        .append("text")
        .attr("text-rendering", "optimizeSpeed")
        .attr("id", `stateLabel${labelData.i}`)
        .attr(
          "transform",
          `translate(${labelData.dx || 0}, ${labelData.dy || 0})`,
        )
        .append("textPath")
        .attr("startOffset", "50%")
        .attr("font-size", `${ratio}%`)
        .node() as SVGTextPathElement;

Comment thread src/renderers/draw-state-labels.ts Outdated
Comment thread src/renderers/draw-state-labels.ts Outdated
Comment thread src/renderers/draw-state-labels.ts
Comment on lines +121 to +123
// Update label data with font size
Labels.update(labelData.i, { fontSize: ratio });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know if these update calls are needed. I have to check that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is currently needed as the editor will use these values.

Comment thread src/renderers/draw-burg-labels.ts Outdated
Comment thread public/modules/dynamic/auto-update.js Outdated
Comment thread src/renderers/draw-custom-labels.ts Outdated
Comment thread src/renderers/draw-custom-labels.ts
Comment thread src/renderers/draw-state-labels.ts
Comment thread src/modules/burgs-generator.ts
@StempunkDev
Copy link
Copy Markdown
Contributor Author

I will go and fix these issues. I have added better burg labels too, I will push that change first, then cleanup

@StempunkDev
Copy link
Copy Markdown
Contributor Author

This should now be everything that I had on my list.

  • Custom Labels now respect groups
  • Added Custom Labels are now added to the pack.labels array
  • Fixed the Id confusion. Label Elements in DOM should now always have there actual id after there typed name.
  • getNextId in Labels now uses a Set to track any free spots in ids to make adding ids faster. (Note: the ids are bound by maxId, pack.labels array does not need to be ordered by id.
  • State Label Editing now allows to edit the actual Name shown in the Label

Is there anything that I missed from a feature standpoint?

I can migrate the label-editor to simple typescript if that is wished for.

@StempunkDev StempunkDev requested a review from Azgaar May 8, 2026 22:50
@Azgaar
Copy link
Copy Markdown
Owner

Azgaar commented May 8, 2026

I will check, but probably not today...

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