-
Notifications
You must be signed in to change notification settings - Fork 77
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
Tab-group switching for snippets #95
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, did not check in action though :-)
Thanks a ton!
@@ -311,7 +311,8 @@ case class SnipDirective(page: Page, variables: Map[String, String]) | |||
} else new File(page.file.getParentFile, source) | |||
val text = Snippet(file, labels) | |||
val lang = Option(node.attributes.value("type")).getOrElse(Snippet.language(file)) | |||
new VerbatimNode(text, lang).accept(visitor) | |||
val group = Option(node.attributes.value("group")).getOrElse("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we should use the tab name for a group if the group is not explicitly defined. This would allow easier feature use for the documentation writers making docs less verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tab name doesn't seem too easy to reach at this point, but defaulting to the lang
seems reasonable, too - applied to #107
: @@snip [example-second.java](../../resources/tab-switching/examples.java) { #java_second group=java } | ||
|
||
Second-scala | ||
: @@snip [example-second.scala](../../resources/tab-switching/examples.scala) { #scala_second group=scala } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIC there is not way to provide grouping for tabs which contain inline text (not a snippet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, would be good to have an approach that supports anything in the tab, rather than relying on the group attribute in snippets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the current tab grouping allow different types of groups? For example, Java/Scala and sbt/Maven/Gradle on the same page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed tabs with inline text are still not supported, but switching arbitrary text is now available in #107.
Different types of groups on the same page should be no problem.
This looks like a great start. One more thing that would be awesome, if we could add support for |
@@ -0,0 +1,3 @@ | |||
@@ snip [Group A](../../test/scala/Multiple.scala) { #multiple-something-else group=a } | |||
|
|||
@@ snip [Group B](../../test/scala/Multiple.scala) { #parseint-imports group=b } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests if the generated HTML is correct, but will the group=b snippet be hidden by default? I have tried it on a project and it looks like that both snippets are always visible.
Also if there are no tabbed snippets in the page, then a user will have no way of switching between the two. I think we should add a hover box or other interactive control that would allow switching between the alternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a selectbox for switching in #107
Actually selecting tabs/alternatives is done in javascript, so isn't easy to incorporate in a test...
Expanding on the proposal in lightbend#95, added support for having other parts of the documentation change as well when changing a group.
Superseded by #107. |
Resolve #11
Resolve #63
Note: update paradox version for paradox in the next release to include the working example in documentation.