revamp iOS settings UI

[incanus]

This was a bit of a yak shave as a result of getting frustrated starting work on #6379 (adding a new runtime styling action), but I think it's long overdue anyway and helps us scale.

The goal here is a reorganization of our iOS settings UI, which is currently an action sheet and here a modal table view. Here's why:

1. The action sheet paradigm dates from a time¹ that only had three actions, all of which would always fit on the screen. Action sheets are ideal for this sort of UI.
2. At current master we now have 18-20 (depending on debugLoggingEnabled) actions. Even if we cull a few, no matter how we slice it, we're going to have to scroll. Might as well use a UI that's designed for it: a table view. We're probably going to keep adding actions.
3. Speaking of adding actions, this separates the UI (section & row labels) from the code that performs the actions, putting the actions all in one routine ². This breaks out some of the inline routines around simple runtime styling, custom callout, custom user dot, and some others. This is more scalable and easier to read.
4. Breaking out runtime styling into individually-labeled actions is better because:
   • Each could possibly benefit from a custom viewport set (out of scope for this here revamp). Right now you need to know that tapping Manipulate Style does like eight things, including styling buildings and removing parks, which might not be relevant to the current viewport, making it look like nothing happened.
   • It's easier to remember what the test UI is capable of with labeled actions right there to remind you, helping us maintain the list going forward.
5. Scrolling any list as long as this action one is a pain. Here I use a group-style table view to break things up a bit by functional task. Now we can add new actions ³ as well as whole new sections ⁴ and things still stay tidy and easy to recall from muscle memory.
6. This avoids the need for a new view controller class by doing some minimal UITableViewController setup along with making our main app view controller the table delegate and dataSource. I started down a different route of settings label & action abstraction in a bigass dictionary, but this is cleaner and is what the delegate pattern is designed for.
7. This actually performs the debug action after the table view completes dismissal. With the action sheet, its requisite animated dismissal sometimes covered up or distracted from the actual debug action being performed. Now you get a clean look at the viewport right before the chosen action happens.
8. This opens up the possibility of new kinds of interaction UI such as sliders, switches, color pickers, etc. for debugging.

/cc @1ec5 @friedbunny @frederoni

[mention-bot]

@incanus, thanks for your PR! By analyzing this pull request, we identified @1ec5, @friedbunny and @boundsj to be potential reviewers.

[incanus]

Grr, of course emoji branch names give Travis trouble. This should be only for core, which this doesn't touch, so I've learned my lesson this time but will let it go for this PR.

[1ec5]

One of the pain points of the current code is that the order is fragile: inserting a new task is prone to off-by-one errors, and Git has a tendency to mangle the order when merging. This PR is an improvement in that the fallout for adding an action is limited to one section rather than the whole list of actions. However, it still relies on literal indices in case statements, which means it’s still fragile around merges. Because -settingsTitlesForSection: and -performActionForSettingAtIndexPath: are separate, these switch statements can still get out of sync if we aren’t vigilant.

If I had followed through on the original rewrite of iosapp in #4221 that introduced the storyboard, I probably would’ve put the action table in the storyboard, hooking each static cell up to a separate IBAction. That way, the workflow for adding an action would be:

1. Add a cell
2. Connect the cell to a new IBAction in the view controller using Connect-to-Source

The IBAction could go anywhere in the source code, and forgetting (2) would just prevent the new action from being functional. There would be a potential for conflicts in the storyboard XML, but conflicts are preferable to botched automerges.

Putting this table in a storyboard can be a future refactor. In the meantime, could you add a mechanism to ensure that the switch statements are in sync, either by replacing the index literals with enum values or adding some assertions?

[incanus]

replacing the index literals with enum values or adding some assertions?

Makes sense @1ec5, I thought about that but you have have good reasons to make it happen. 👍

[1ec5]

#6401 tracks the storyboard approach proposed in #6389 (review).

[1ec5]

• Add a section to Contributing that explains how to add a new action to iosapp

[incanus]

I'm not concerned about the storyboard in #6401 yet since we don't add actions that often, but I replaced the integers with NS_ENUM stuff and rebased above. If this looks good @1ec5 I'll 🚢 once tests pass.

[1ec5]

Should be this an assertion, in case a contributor forgets to add a case here or adds it to the wrong switch statement?

[incanus]

Or a thrown exception?

[1ec5]

This is in iosapp, so there’s no need to throw exceptions. If this were SDK code, an exception would be a fine idea.

[incanus]

Handled in the rebased 80079729e8d8fbab67ec08f557f8d4392a77a348.

[incanus]

I think I'm going to pass on:

• Add a section to Contributing that explains how to add a new action to iosapp

It should be pretty apparent and this document is more geared towards SDK contributions, not the test app.

[incanus]

To reiterate, Travis fails here because of the Emoji branch name. Lesson learned. It's all non-Cocoa tests, though, which this PR doesn't impact, so going to proceed with merge anyway.

[1ec5]

Revert this.

[incanus]

Whoops.

[incanus]

479919b3cb36ebbd7b7502c6c7ba457e1d5628cb

[1ec5]

Eh? Probably worth breaking out into a different PR if you don’t like the default (but disabled) breakpoint. 😉

[incanus]

I feel like early on this was something I reversed in master during this process. I'll take it out of this PR; didn't mean to change anything currently.

[incanus]

Ok, one more time with this 🎪 in 8c5ccff!

[incanus]

👉 #6421 to avoid Emoji pain.