Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show layout names rather than keys in Preferences UI #792

Merged
merged 4 commits into from Feb 25, 2019

Conversation

cdisselkoen
Copy link
Contributor

Also rename some variables/functions to explicitly distinguish between layout keys and
names (avoid ambiguous term layoutString). If you want, I can split this into two separate PRs, but I figured they are quite related.

This should be a UI change only; functionality should not be affected. For instance, preferences storage is still by layout key, not layout name; it's just displayed as layout names.

With this PR, the layout keys should be entirely invisible to the average user, unless I'm missing something. This is advantageous for layouts where the key bears little resemblance to the name, e.g. key middle-wide with name 3Column Middle.

Also rename some variables/functions to distinguish between keys and
names
Copy link
Owner

@ianyh ianyh left a comment

Choose a reason for hiding this comment

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

This is a great change. A couple comments, but overall looks good.

BinarySpacePartitioningLayout.self
]

static var layoutByKey: [String: Layout.Type] = Dictionary(uniqueKeysWithValues: zip( layoutClasses.map { ($0.layoutKey) }, layoutClasses ))

Choose a reason for hiding this comment

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

Line Length Violation: Line should be 120 characters or less: currently 144 characters (line_length)

@cdisselkoen
Copy link
Contributor Author

The Travis build failed with The Xcode build system has crashed; I'm not sure what happened there. This branch still works on my machine.

@ianyh
Copy link
Owner

ianyh commented Feb 25, 2019

I reran it. It seems like it was a transient Travis failure and not the tests.

@ianyh ianyh merged commit 43a7c12 into ianyh:development Feb 25, 2019
@cdisselkoen cdisselkoen deleted the names-in-prefs branch February 25, 2019 23:50
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.

None yet

3 participants