fix(plugins/plugin-client-common): nested choices in wizards were not presented as such

We weren't handling nested choice at all. As a result, the number of "Tasks" that Kui suggested the user needed to complete, both at the top level (in the progress bar in the wizard header) and also in the number of little squares in the given step, were both off.

Author: starpit

plugins/plugin-client-common/src/components/Content/Markdown/components/Wizard/CodeBlockProps.ts

@@ -30,14 +30,17 @@ export default interface CodeBlockProps {
      * This option names the group, to keep it distinct from other
      * groups of choices.
      */
-    group?: string
+    group: string
 
     /**
      * This option names that member. e.g. if the user can choose
      * between doing either A-and-B or C-and-D, this identifies
      * whether we are part ofth e first choice (A+B) or the second
      * (C+D).
      */
-    member?: string
+    member: number
+
+    /** Is this a nested choice? */
+    nestingDepth: number
   }
 }

plugins/plugin-client-common/src/components/Content/Markdown/components/Wizard/index.tsx

@@ -21,7 +21,7 @@ import { WizardProps } from './rehype-wizard'
 import Progress from './Progress'
 import CodeBlockProps from './CodeBlockProps'
 import { onTabSwitch, offTabSwitch } from '../tabbed'
-import { OrderedGraph, blocks, compile, order, sequence } from '../code/graph'
+import { ChoicesMap, OrderedGraph, blocks, compile, order, sequence } from '../code/graph'
 
 import Card from '../../../../spi/Card'
 import Icons from '../../../../spi/Icons'
@@ -52,7 +52,7 @@ export interface State {
   status: Record<string, Status>
 
   /** Map from tab group to currently selected tab member */
-  choices: Record<string, string>
+  choices: ChoicesMap
 }
 
 export default class Wizard extends React.PureComponent<Props, State> {

plugins/plugin-client-common/src/components/Content/Markdown/components/code/graph.ts

@@ -31,7 +31,7 @@ import { ProgressStepState } from '../../../ProgressStepper'
 type Status = ProgressStepState['status']
 
 /* map from choice group to selected choice member */
-type ChoicesMap = Record<CodeBlockProps['choice']['group'], CodeBlockProps['choice']['member']>
+export type ChoicesMap = Record<CodeBlockProps['choice']['group'], CodeBlockProps['choice']['member']>
 
 /** Iteration order */
 export interface Ordered {
@@ -48,7 +48,7 @@ type Choice<T extends Unordered | Ordered = Unordered> = {
   group: string
 
   choices: {
-    member: string
+    member: number
     graph: Sequence<T>
   }[]
 }
@@ -100,15 +100,24 @@ export function compile(blocks: CodeBlockProps[], ordering: 'sequence' | 'parall
 
   let currentChoices: Choice
   let currentChoice: CodeBlockProps['choice']
+  const currentChoiceStack: { currentChoice: CodeBlockProps['choice']; currentChoices: Choice }[] = []
 
   const parts: Graph[] = []
 
   blocks.forEach(block => {
     if (block.choice) {
-      if (!currentChoice || currentChoice.group !== block.choice.group) {
-        currentChoice = block.choice
+      if (
+        currentChoiceStack.length > 0 &&
+        currentChoice.nestingDepth > 0 &&
+        block.choice.nestingDepth < currentChoice.nestingDepth
+      ) {
+        const tmp = currentChoiceStack.pop()
+        currentChoice = tmp.currentChoice
+        currentChoices = tmp.currentChoices
+      }
 
-        currentChoices = {
+      if (!currentChoice || currentChoice.group !== block.choice.group) {
+        const newChoice = {
           group: block.choice.group,
           choices: [
             {
@@ -118,7 +127,21 @@ export function compile(blocks: CodeBlockProps[], ordering: 'sequence' | 'parall
           ]
         }
 
-        parts.push(currentChoices)
+        const isNested = currentChoice && block.choice.nestingDepth > 0 && block.choice.member === 0
+        if (isNested) {
+          currentChoiceStack.push({ currentChoice, currentChoices })
+          currentChoices.choices.push({
+            member: currentChoices.choices.length,
+            graph: sequence([newChoice])
+          })
+        }
+
+        currentChoice = block.choice
+        currentChoices = newChoice
+
+        if (!isNested) {
+          parts.push(currentChoices)
+        }
       } else if (currentChoice.member === block.choice.member) {
         // continuation of the current choice member
         currentChoices.choices[currentChoices.choices.length - 1].graph.sequence.push(block)

plugins/plugin-client-common/src/components/Content/Markdown/components/code/rehype-code-indexer.ts

@@ -90,23 +90,14 @@ export default function plugin(uuid: string) {
                   reserialize()
                 }
 
-                if (
-                  !attributes.cleanup &&
-                  !attributes.validate &&
-                  attributes.optional !== false &&
-                  isImplicitlyOptional(_)
-                ) {
-                  // don't propagate code blocks out of implicitly
-                  // optional elements, unless the code block has an
-                  // associated validator. The idea is that if the
-                  // author went through the trouble of writing a
-                  // validator for a code block, it probably isn't
-                  // optional. If the absence of a validator, then
-                  // we should stop propagating the code block
-                  // upwards if we reach some element that seems
-                  // indicative of blocking out an optional area,
-                  // e.g. an expandable section that is
-                  // default-closed
+                if (attributes.optional === true || isImplicitlyOptional(_)) {
+                  // don't propagate code blocks out of either
+                  // explicitly or implicitly optional elements. Re:
+                  // implicitly, the idea is that we should stop
+                  // propagating the code block upwards if we reach
+                  // some element that seems indicative of blocking
+                  // out an optional area, e.g. an expandable section
+                  // that is default-closed
                   attributes.optional = true
                   reserialize()
                 }
@@ -118,16 +109,19 @@ export default function plugin(uuid: string) {
                       const parent = ancestors[idx - 1]
                       if (isElementWithProperties(parent)) {
                         const group = parent.properties['data-kui-choice-group']
-                        const member = String(_.properties['data-kui-tab-index'])
+                        const member = parseInt(_.properties['data-kui-tab-index'].toString(), 0)
+                        const nestingDepth = parseInt(parent.properties['data-kui-choice-nesting-depth'].toString(), 0)
 
                         if (!attributes.choice) {
                           attributes.choice = {
                             group,
-                            member
+                            member,
+                            nestingDepth
                           }
                         } else {
                           attributes.group = group
                           attributes.member = member
+                          attributes.nestingDepth = nestingDepth
                         }
 
                         // re-serialize this update for later use

plugins/plugin-client-common/src/components/Content/Markdown/rehype-tabbed/index.ts

@@ -45,7 +45,8 @@ export default function plugin(/* options */) {
           children: currentTabs,
           properties: {
             depth: tabStack.length,
-            'data-kui-choice-group': v4()
+            'data-kui-choice-group': v4(),
+            'data-kui-choice-nesting-depth': tabStack.length
           }
         })
       }

plugins/plugin-client-common/src/test/core/markdown/wizards.ts

@@ -83,7 +83,37 @@ const IN4 = {
     { name: 'CCC', body: 'CCCContent', description: '', codeBlocks: [{ index: 2, output: '333' }] }
   ]
 }
-;[IN4, IN1, IN2, IN3].forEach(markdown => {
+
+// nested choice
+const IN5 = {
+  input: join(ROOT, 'tests/data/nested-choice1.md'),
+  title: 'WizardTitle',
+  description: 'WizardDescription',
+  expectedSplitCount: 1,
+  expectedCodeBlockTasks: 1,
+  steps: [{ name: 'AAA', body: 'AAAContent', description: '', codeBlocks: [{ index: 0, output: 'XXX' }] }]
+}
+
+// nested choice
+const IN6 = {
+  input: join(ROOT, 'tests/data/nested-choice2.md'),
+  title: 'WizardTitle',
+  description: 'WizardDescription',
+  expectedSplitCount: 1,
+  expectedCodeBlockTasks: 2,
+  steps: [
+    {
+      name: 'AAA',
+      body: 'AAAContent',
+      description: '',
+      codeBlocks: [
+        { index: 0, output: 'XXX1' },
+        { index: 1, output: 'XXX2' }
+      ]
+    }
+  ]
+}
+;[IN1, IN2, IN3, IN4, IN5, IN6].forEach(markdown => {
   describe(`wizards in markdown ${basename(markdown.input)} ${process.env.MOCHA_RUN_TARGET ||
     ''}`, function(this: Common.ISuite) {
     before(Common.before(this))

plugins/plugin-client-common/tests/data/nested-choice1.md

@@ -0,0 +1,27 @@
+---
+layout: wizard
+---
+
+# WizardTitle
+
+WizardDescription
+
+---
+
+## AAA
+
+AAAContent
+
+=== "Tab11"
+
+    === "Tab21"
+        ```bash
+        echo XXX
+        ```
+
+    === "Tab22"
+        ```bash
+        echo YYY
+        ```
+
+=== "Tab12"

plugins/plugin-client-common/tests/data/nested-choice2.md

@@ -0,0 +1,31 @@
+---
+layout: wizard
+---
+
+# WizardTitle
+
+WizardDescription
+
+---
+
+## AAA
+
+AAAContent
+
+=== "Tab11"
+
+    === "Tab21"
+        ```bash
+        echo XXX1
+        ```
+
+        ```bash
+        echo XXX2
+        ```
+
+    === "Tab22"
+        ```bash
+        echo YYY
+        ```
+
+=== "Tab12"