Skip to content

POI: hide Attributes for local geometry; All Tags header tap targets#959

Closed
tordans wants to merge 2 commits into
bryceco:masterfrom
tordans:cursor/poi-tab-remember-and-all-tags-header-a5db
Closed

POI: hide Attributes for local geometry; All Tags header tap targets#959
tordans wants to merge 2 commits into
bryceco:masterfrom
tordans:cursor/poi-tab-remember-and-all-tags-header-a5db

Conversation

@tordans

@tordans tordans commented May 31, 2026

Copy link
Copy Markdown
Contributor

Problem

This changes two things at once because I mixed up my instructions. But they are related, so maybe they can stay in one PR.

Hide Attributes for local / new geometry

Issue 1: The 3rd tab "Attributes" was hidden for new nodes but not for new ways/areas

POITabBarController restores UserPrefs.poiTabIndex when the POI editor opens (0 = Common Tags, 1 = All Tags, 2 = Attributes).

For a new standalone node (selectedPrimary == nil), the Attributes tab is already removed and a saved index of 2 is clamped to 0.

For a new way / area / relation in memory (ident < 0 until upload), selectedPrimary is non-nil, so Attributes used to stay visible and index 2 could still be restored—inconsistent with the new-node experience.

User story: After editing an uploaded object on Attributes, opening the editor for any not-yet-uploaded geometry (nil selection or local negative id) should match a new node: only Common Tags and All Tags; Attributes returns after the object has a positive server id.

Consolidation: PR #6 addressed the same Attributes-tab bug. This PR keeps that fix plus the All Tags header work below. PR #6 is closed as superseded; unit tests from #6 are included here (POITabBarControllerTestCase).

All Tags preset header

Issue 2a: The "change/choose preset" arrow on the "All Tags" did not look that great
Issue 2b: When the keyboard was up (which happens automatically in some flows), there is no direct way to change from "All Tags" to "Common Tags" => This is now solved by making the Preset label clickable

On All Tags, the section header shows the matched preset name and a control to open the feature picker. When the keyboard hides the tab bar, users could not easily return to Common Tags preset fields. The trailing control was plain ">" text.

User story: Tap the preset title → Common Tags; tap the chevron → change preset (unchanged behavior).


Implementation notes (Cursor)

POITabBarController.swift

  • shouldHideAttributesTab(for:)true when selection == nil or ident < 0.
  • resolvedTabBar(savedIndex:selection:) — tab count 2 vs 3; clamps saved index 2 → 0 when Attributes hidden.
  • viewDidLoad uses resolvedTabBar; removeAttributesTabFromViewControllers() strips the Attributes nav stack.

POIAllTagsViewController.swift (SectionHeaderCell)

  • Title: slideTabTo(tabIndex: 0) + updates poiTabIndex; does not open feature picker.
  • Trailing: chevron.right, accessibility “Change preset”; pushes POIFeaturePickerViewController.
  • UIButton.Configuration on iOS 15+; fallback on older OS.

Tests (from PR #6)

  • POITabBarControllerTestCase — matrix for savedIndex ∈ {0,1,2} × {nil, pending node, uploaded node, pending way}.

Out of scope: Refreshing tabs if selection goes pending → uploaded while POI stays open.


Testing notes (@tordans)

POI tab / Attributes visibility

  1. Uploaded node → Attributes → then new node (pushpin) → two tabs only; if last index was 2, land on Common Tags.
  2. Same with last session on Attributes (index 2) before new node → Common Tags, no Attributes.
  3. New way/area (ident < 0) → POI → Attributes hidden; index 2 does not select a missing third tab.
  4. After upload or existing object (ident > 0) → three tabs; index 2 restores normally.

All Tags header

  1. Tags present → All Tags → keyboard up → tap header titleCommon Tags.
  2. Tap chevron → feature picker; title must not open picker.
  3. VoiceOver: two elements; “Change preset” on chevron.
  4. iPad / Mac Catalyst: trailing control ≥ 44pt.
BeforeAfter
tags-panels--hide-attributes--switcher--before.min.mov
tags-panels--hide-attributes--switcher--after.min.mov

Automated: Run GoMapTestsPOITabBarControllerTestCase.

Open in Web Open in Cursor 

cursoragent and others added 2 commits May 30, 2026 15:50
- Treat pending geometry (ident < 0) like new nodes when restoring poiTabIndex:
  strip Attributes tab and clamp saved index 2 to Common Tags.
- All Tags preset header: title switches to Common Tags; chevron opens picker.

Co-authored-by: Tobias <t@tobiasjordans.de>
Expose resolvedTabBar for tab index clamping; adopt in viewDidLoad.
Supersedes duplicate Attributes-tab logic in draft PR #6.

Co-Authored-By: Tobias <t@tobiasjordans.de>
@bryceco

bryceco commented Jun 6, 2026

Copy link
Copy Markdown
Owner

Switching from All Tags to Common Tags when the keyboard is showing:

  • The approach you chose seems really hard to discover, and also counterintuitive. Maybe another button like the "Previous/Next" buttons we show in the input accessory view (prevNextToolbar)?
  • It also bugs me that this solves the issue from All -> Common but not the other direction.

Attributes tab:

  • I'm undecided on this. If the goal is consistency we could create a dummy object that the Attributes page could reference to fill in the fields.

@tordans

tordans commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Switching from All Tags to Common Tags when the keyboard is showing:

  • The approach you chose seems really hard to discover, and also counterintuitive. Maybe another button like the "Previous/Next" buttons we show in the input accessory view (prevNextToolbar)?
  • It also bugs me that this solves the issue from All -> Common but not the other direction.

I agree this is not perfect. One thing that does not help is the animations, which I did not look into. There is some flicker and inconsistency that make this harder to understand.

Unfortunately I have yet to come up with a better idea on how to solve this issue in an iOS like way.
The crux is, that when the keyboard is hidden, the UI works well. And at the same time, we want the keyboard to be open by default in those flows. But the "hide keyboard to get to the buttons to switch the panel" flow is counter intuitive as well and I don't get good muscle memory going for it…

Idea: Maybe we can create a new floating UI that is similar to the 3-tab-switch that we see when the keyboard is down but only visible when the keyboard is up. It would have to be smaller in height though to not take up too much space…
The new Glass-UI-Design makes this more feasable.
On https://developer.apple.com/design/new-design-gallery/ the very last example from Slack might be a direction

  • Maybe a top tab bar when keyboard is up that looks similar to the bottom tab bar
    image
  • Maybe a smaller tab bar above the keyboard
    image

The UI Guidelines actually help with this:

IMG_1782 min

Attributes tab:

  • I'm undecided on this. If the goal is consistency we could create a dummy object that the Attributes page could reference to fill in the fields.

My thinking is:

  • Its great that the UI remembers my last panel choise
  • The attribute panel is never useful when objects are new; so hiding it is best
  • The panel already hides attributes for new nodes and handles the case about my last panel choise

=> We should align all geometry types with how nodes work now.

@bryceco

bryceco commented Jun 6, 2026

Copy link
Copy Markdown
Owner

use icon only for prev/next,
use the icon only UI of the tabs on the left

Yes, thats what we should do.

The toolbar you see in Slack is like what we use for telephone # input.

@tordans tordans marked this pull request as draft June 6, 2026 07:14
@tordans

tordans commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Yes, thats what we should do.

Happy to update this. But might be a while until I can shift focus on this.
Will split out the other topic then, so those are easier to discuss.

@bryceco

bryceco commented Jun 7, 2026

Copy link
Copy Markdown
Owner

I'm working on the keyboard helper with options to switch tabs, and that will be a different PR. This one is too mixed up with different goals, so I think it should be closed. New PRs with individual goals for what's left over are fine.

@bryceco bryceco closed this Jun 7, 2026
@tordans tordans deleted the cursor/poi-tab-remember-and-all-tags-header-a5db branch June 9, 2026 05:09
@tordans

tordans commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Attributes tab:

  • I'm undecided on this. If the goal is consistency we could create a dummy object that the Attributes page could reference to fill in the fields.

My thinking is:

  • Its great that the UI remembers my last panel choise
  • The attribute panel is never useful when objects are new; so hiding it is best
  • The panel already hides attributes for new nodes and handles the case about my last panel choise

=> We should align all geometry types with how nodes work now.

FYI I extracted this part at tordans#19 – Once I have time to test it, I will propose a new PR for discussion.

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