Skip to content

Attributes tab: Done dismisses POI editor when tags unchanged#955

Open
tordans wants to merge 1 commit into
bryceco:masterfrom
tordans:cursor/attributes-done-dismiss-7085
Open

Attributes tab: Done dismisses POI editor when tags unchanged#955
tordans wants to merge 1 commit into
bryceco:masterfrom
tordans:cursor/attributes-done-dismiss-7085

Conversation

@tordans
Copy link
Copy Markdown
Contributor

@tordans tordans commented May 31, 2026

Problem

On POI editor tabs (Common Tags, All Tags, Attributes), the navigation Done (checkmark) was disabled when tags were unchanged (saveButton.isEnabled = false). A disabled bar button does not run done(_:), so the checkmark could highlight but the sheet stayed open. With edits, Done was enabled (blue) and dismissed + saved as expected.

Expected: Gray checkmark when unchanged, but tap still dismisses without saving. Blue checkmark when there are edits: save and dismiss.

Why: This allows users to aways close the panel with the top right button which is easer to remember and easer to reach. Also when nothing changed, using "save changes" is OK because it changes nothing…

Implementation notes (by Cursor)

Root cause: UIBarButtonItem.isEnabled = false blocks the action selector.

Fix:

  • Added POITabBarController.updateSaveButton(_:hasUnsavedTagChanges:) — Done stays enabled (tappable); tint is gray (.tertiaryLabel) when unchanged, default accent when changed; isModalInPresentation still tracks unsaved changes.
  • done(_:) on Common Tags, All Tags, and Attributes: always dismiss, call commitChanges() only when isTagDictChanged().
  • Replaced prior saveButton.isEnabled = isTagDictChanged() call sites in those controllers with updateSaveButton.

Note: New objects hide the Attributes tab; the behavior you see on “tags panel” is Common Tags (first tab).

Automated testing: Not covered by XCTest in this PR.

Testing notes (@tordans)

  1. New node → Common Tags (default) → gray checkmark → tap → sheet closes, no tags written.
  2. Existing node, no edits → same.
  3. Edit a tag → checkmark blue → tap → sheet closes, tags saved.
  4. Attributes tab (existing object only): unchanged → gray, dismiss only; changed (from another tab) → blue, dismiss + commit.
  5. Cancel still dismisses without commit.
BeforeAfter
ok-dismiss--before.min.mov
ok-dismiss--after.min.mov

When the tag dictionary matches the map object, Done now dismisses
without calling commitChanges(). When tags changed, behavior is unchanged:
dismiss then commit. Removed viewWillAppear logic that disabled Done when
there were no edits.

Fix Done dismiss when tags unchanged on Common/All Tags tabs

Disabled UIBarButtonItem does not invoke done:, so gray checkmark could
highlight without closing. Keep Done enabled and use tint for gray/blue;
only commit when isTagDictChanged(). Shared updateSaveButton on
POITabBarController.

Co-Authored-By: Tobias <t@tobiasjordans.de>
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.

2 participants