Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Indicate protocols, abstract classes in documentation navigation #7478

Merged
merged 2 commits into from
Dec 21, 2016

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Dec 19, 2016

Italicize protocols and abstract classes in the generated documentation’s navigation sidebar. This treatment hopefully gives the concrete classes more prominence while keeping concrete classes together with their abstract superclasses:

abstract

This visual distinction doesn’t necessarily preclude the “Protocols and Abstract Classes” and “Style Layers (Abstract)” categories being added in #7338, although it may make them less necessary. (I do agree that we need to reorganize the categories regardless.) I think keeping concrete classes together with their abstract superclasses in navigation is important, because classes very often inherit initializers and properties from their superclasses. Until realm/jazzy#708 lands, it’s a bit difficult otherwise to navigate from a class to its superclass.

I didn’t bother to pull in Open Sans Italic for just a handful of links, although that’s certainly a possibility. Alternatively, we could denote protocols and abstract classes with faint “(Protocol)” and “(Abstract Class)” glosses or perhaps [P] and [A] icons. Or we could even surround the protocol and abstract class names with parentheses.

Unfortunately, since there’s nothing in Objective-C that would distinguish an abstract class from a concrete class, we have to maintain a hard-coded list of abstract classes in jazzy.css.scss. Ideally we’d list these classes in jazzy.yml or something, I guess.

/cc @mayagao @ericrwolfe

@1ec5 1ec5 added documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Dec 19, 2016
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Dec 19, 2016
@1ec5 1ec5 self-assigned this Dec 19, 2016
@mention-bot
Copy link

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

@1ec5 1ec5 requested a review from friedbunny December 19, 2016 01:03
@friedbunny
Copy link
Contributor

friedbunny commented Dec 19, 2016

I very much like the idea of differently emphasizing abstract classes and protocols, but we can improve on the design. 😉 Fauxtalics are a bit distracting here and over-emphasize the importance of these classes/protocols — something more subtle seems like the way to go, if we can manage that while still conveying meaning.

Perhaps you have some suggestions, @mayagao?

.nav-group-task[data-name="MGLSource"],
.nav-group-task[data-name="MGLStyleLayer"],
.nav-group-task[data-name="MGLTileSource"],
.nav-group-task[data-name="MGLVectorStyleLayer"] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Embedding bits of our SDK into the jazzy theme’s CSS feels mildly icky — could this be scripted or upstreamed to jazzy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’d like that very much. What do you think jazzy should expose as an option, given that the idea of an “abstract class” is very much a user-defined concept, not something baked into the language? Perhaps jazzy could allow you to attach arbitrary properties to items in the custom_categories configuration variable?

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 19, 2016

Fauxtalics are a bit distracting here and over-emphasize the importance of these classes/protocols — something more subtle seems like the way to go, if we can manage that while still conveying meaning.

If oblique text (that’s the term of art) is too unsubtle, here are some subtler alternatives:

gloss

parens

But I should caution that these classes aren’t necessarily unimportant: an abstract class is more important than any given concrete subclass; it just shouldn’t be a developer’s starting point.

@mayagao
Copy link
Contributor

mayagao commented Dec 20, 2016

Yeah agree with @friedbunny the faux italic looks really weird and I would avoid using it. I guess the main question is, is the distinction between protocols and abstract classes really important when people are browsing the list? Or is it something we can highlight in the details section on the right?

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 20, 2016

is the distinction between protocols and abstract classes really important when people are browsing the list? Or is it something we can highlight in the details section on the right?

We already state in the details that it’s a protocol or abstract class. This fact is important enough that #7338 originally would’ve split out a separate category for protocols and abstract classes, which I thought swung a bit too far in the other direction.

Xcode’s Documentation window sets off protocols into a separate section and gives every item an icon. But I think Xcode’s treatment works better for frameworks that don’t rely so heavily on abstract classes to provide functionality. Also, Xcode groups topics only by their type (class versus protocol), not by task like we do.

xcode

@mayagao
Copy link
Contributor

mayagao commented Dec 20, 2016

I wonder how other documentations deal with, but we can merge this for now and iterate later!

1ec5 added 2 commits December 20, 2016 16:46
Italicize protocols and abstract classes in the generated documentation’s navigation sidebar.
Instead of oblique text, use a parenthetical gloss to denote a protocol or abstract class.
@1ec5 1ec5 force-pushed the 1ec5-docs-abstract branch from dcadb57 to 23599e0 Compare December 21, 2016 00:46
@1ec5
Copy link
Contributor Author

1ec5 commented Dec 21, 2016

Per discussion with @mayagao, I ended up adding a parenthetical gloss after each protocol and abstract class instead of obliquing the protocol or class name.

@1ec5 1ec5 changed the title Italicize protocols, abstract classes in documentation navigation Indicate protocols, abstract classes in documentation navigation Dec 21, 2016
@1ec5 1ec5 merged commit 906d700 into release-ios-v3.4.0 Dec 21, 2016
@1ec5 1ec5 deleted the 1ec5-docs-abstract branch December 21, 2016 01:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants