ronantakizawa/codereview-bench huggingface dataset:
Repository: h2oai/wave
File: ui/src/choice_group.tsx (TypeScript)

Code before:
export const
  XChoiceGroup = ({ model: m }: { model: ChoiceGroup }) => {
    const
      options = (m.choices || []).map(({ name, label, disabled }): Fluent.IChoiceGroupOption => ({ key: name, text: label || name, disabled })),
      onChange = (_e?: React.FormEvent<HTMLElement>, option?: Fluent.IChoiceGroupOption) => {
        if (option) wave.args[m.name] = option.key
        if (m.trigger) wave.push()
      },
      styles: Fluent.IChoiceGroupStyles = {
        flexContainer: {
          display: 'flex',
          flexWrap: 'wrap',
          selectors: {
            '.ms-ChoiceField': {
              marginRight: 15,
            }
          }
        },
      }

    // eslint-disable-next-line react-hooks/exhaustive-deps
    React.useEffect(() => { wave.args[m.name] = m.value || null }, [])

    return (
      <Fluent.ChoiceGroup
        styles={styles}
        data-test={m.name}
        label={m.label}
        required={m.required}
        defaultSelectedKey={m.value}
        options={options}
        onChange={onChange}
      />
    )

  }

Review comment (suggestion):
```suggestion
        styles={{ flexContainer: { display: 'flex', flexWrap: 'wrap' } }}
```

Code after:
export const
  XChoiceGroup = ({ model: m }: { model: ChoiceGroup }) => {
    const
      optionStyles = { choiceFieldWrapper: { marginRight: 15 } },
      options = (m.choices || []).map(({ name, label, disabled }): Fluent.IChoiceGroupOption => ({ key: name, text: label || name, disabled, styles: optionStyles })),
      onChange = (_e?: React.FormEvent<HTMLElement>, option?: Fluent.IChoiceGroupOption) => {
        if (option) wave.args[m.name] = option.key
        if (m.trigger) wave.push()
      }

    // eslint-disable-next-line react-hooks/exhaustive-deps
    React.useEffect(() => { wave.args[m.name] = m.value || null }, [])

    return (
      <Fluent.ChoiceGroup
        styles={{ flexContainer: { display: 'flex', flexWrap: 'wrap' } }}
        data-test={m.name}
        label={m.label}
        required={m.required}
        defaultSelectedKey={m.value}
        options={options}
        onChange={onChange}
      />
    )

  }
Repository: h2oai/wave
File: ui/src/table.tsx (TypeScript)

Code before:
  tooltip?: S
}

type WaveColumn = Fluent.IColumn & {
  dataType?: S
  cellType?: TableCellType
  isSortable?: B
}

type DataTable = {
  model: Table
  onFilterChange: (filterKey: S, filterVal: S, checked?: boolean) => void
  sort: (col: WaveColumn) => void,
  reset: () => void
  filteredItems: any[]
  selectedFilters: Dict<S[]> | null
  items: any[]
  selection: Fluent.Selection
  isMultiple: B
  isSearchable: B
  groups?: Fluent.IGroup[]
  setFiltersInBulk: (colKey: S, filters: S[]) => void
}

type ContextualMenuState = {
  onFilterChange: (filterKey: S, filterVal: S, checked?: boolean) => void
  col: WaveColumn
  listProps: Fluent.IContextualMenuListProps
  selectedFiltersRef: React.MutableRefObject<Dict<S[]> | null>
  setFiltersInBulk: (colKey: S, filters: S[]) => void
}

const
  MIN_ROWS_TO_DISPLAY_FOOTER = 20,
  // TODO: Clean up into correct Fluent style slots.
  css = stylesheet({
    // HACK: Put sorting icon on right (same as filter).
    sortableHeader: {
      $nest: {
        '.ms-DetailsHeader-cellName': {
          position: 'relative',
          paddingRight: 15
        }
      }
    },
    sortingIcon: {
      marginLeft: 10,
      fontSize: rem(1.1),
      position: 'absolute',
      top: -2,
      right: -5

Review comment (suggestion):
nit
```suggestion
  onFilterChange: (filterKey: S, filterVal: S, checked?: B) => void
```

Code after:
  tooltip?: S
}

type WaveColumn = Fluent.IColumn & {
  dataType?: S
  cellType?: TableCellType
  isSortable?: B
}

type DataTable = {
  model: Table
  onFilterChange: (filterKey: S, filterVal: S, checked?: B) => void
  sort: (col: WaveColumn) => void,
  reset: () => void
  filteredItems: any[]
  selectedFilters: Dict<S[]> | null
  items: any[]
  selection: Fluent.Selection
  isMultiple: B
  isSearchable: B
  groups?: Fluent.IGroup[]
  setFiltersInBulk: (colKey: S, filters: S[]) => void
}

type ContextualMenuProps = {
  onFilterChange: (filterKey: S, filterVal: S, checked?: B) => void
  col: WaveColumn
  listProps: Fluent.IContextualMenuListProps
  selectedFiltersRef: React.MutableRefObject<Dict<S[]> | null>
  setFiltersInBulk: (colKey: S, filters: S[]) => void
}

const
  MIN_ROWS_TO_DISPLAY_FOOTER = 20,
  // TODO: Clean up into correct Fluent style slots.
  css = stylesheet({
    // HACK: Put sorting icon on right (same as filter).
    sortableHeader: {
      $nest: {
        '.ms-DetailsHeader-cellName': {
          position: 'relative',
          paddingRight: 15
        }
      }
    },
    sortingIcon: {
      marginLeft: 10,
      fontSize: rem(1.1),
      position: 'absolute',
      top: -2,
      right: -5
Repository: h2oai/wave
File: ui/src/tooltip.tsx (TypeScript)

Code before:
      textAlign: 'left',
      marginLeft: '0.5em',
      cursor: 'pointer'
    },
    preventOverflow: {
      minWidth: 0,
    }
  })

export const
  XToolTip = ({ children, content, showIcon = true, expand = true }: {
    children: React.ReactChild,
    content?: S,
    showIcon?: B,
    expand?: B
  }) => {
    if (!content) return <>{children}</>

    const tooltipProps: Fluent.ITooltipProps = { onRenderContent: () => <div><Markdown source={content} /></div> }
    return (
      <div className={css.container} data-test='tooltip'>
        {
          showIcon
            ? (
              <>
                <div className={clas(css.element, expand ? '' : css.preventOverflow )}>{children}</div>
                <Fluent.TooltipHost tooltipProps={tooltipProps}>
                  <Fluent.FontIcon className={css.icon} iconName='Info' />
                </Fluent.TooltipHost>
              </>
            )
            : <Fluent.TooltipHost tooltipProps={tooltipProps}>{children}</Fluent.TooltipHost>
        }
      </div>
    )
  }

Review comment (bug):
Shouldn't it be 
```suggestion
                <div className={expand ? css.element : css.preventOverflow}>{children}</div>
```
according to the previous logic?

Are all the other form components checked for possible regressions? Both with and without tooltip.

Edit: Across major browsers as well.

Code after:
      textAlign: 'left',
      marginLeft: '0.5em',
      cursor: 'pointer'
    },
    preventOverflow: {
      minWidth: 0
    }
  })

export const
  XToolTip = ({ children, content, showIcon = true, expand = true }: {
    children: React.ReactChild,
    content?: S,
    showIcon?: B,
    expand?: B
  }) => {
    if (!content) return <>{children}</>

    const tooltipProps: Fluent.ITooltipProps = { onRenderContent: () => <div><Markdown source={content} /></div> }
    return (
      <div className={css.container} data-test='tooltip'>
        {
          showIcon
            ? (
              <>
                <div className={clas(css.preventOverflow, expand ? css.expand : '')}>{children}</div>
                <Fluent.TooltipHost tooltipProps={tooltipProps}>
                  <Fluent.FontIcon className={css.icon} iconName='Info' />
                </Fluent.TooltipHost>
              </>
            )
            : <Fluent.TooltipHost tooltipProps={tooltipProps}>{children}</Fluent.TooltipHost>
        }
      </div>
    )
  }
Repository: h2oai/wave
File: ui/src/markdown_table_cell_type.tsx (TypeScript)

Code before:
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import { S } from 'h2o-wave'
import React from 'react'
import { Markdown } from './markdown'

/**
 * Create a cell type that renders Markdown content.
*/
export interface MarkdownTableCellType {
  /** An identifying name for this component. */
  name?: S
  /** Where to display the link. Setting this to '_blank'` opens the link in a new tab or window. */
  target?: S
}

export const XMarkdownTableCellType = ({ model: m }: { model: MarkdownTableCellType & { content: S}} ) => {
  const ref = React.useRef<HTMLDivElement>(null)

  React.useEffect(() => {
    const anchors = document.querySelectorAll<HTMLAnchorElement>('a')
    anchors?.forEach(a => {if (m.target) a.target = m.target})
  }, [m.target])

  return (
    <div data-test={m.name} ref={ref}>
      <Markdown source={m.content} />
    </div>
  )
}

Review comment (suggestion):
This changes all the links across the whole document which we probably do not want. Let's instead try something like:

```suggestion
    ref.current?.querySelectorAll<HTMLAnchorElement>('a')?.forEach(a => {if (m.target) a.target = m.target})
```

Code after:
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import { S } from 'h2o-wave'
import React from 'react'
import { Markdown } from './markdown'

/**
 * Create a cell type that renders Markdown content.
*/
export interface MarkdownTableCellType {
  /** An identifying name for this component. */
  name?: S
  /** Where to display the link. Setting this to '_blank'` opens the link in a new tab or window. */
  target?: S
}

export const XMarkdownTableCellType = ({ model: m }: { model: MarkdownTableCellType & { content: S}} ) => {
  const ref = React.useRef<HTMLDivElement>(null)

  React.useEffect(() => {
    ref.current!.querySelectorAll<HTMLAnchorElement>('a')?.forEach(a => {if (m.target) a.target = m.target})
  }, [m.target])

  return (
    <div data-test={m.name} ref={ref}>
      <Markdown source={m.content} />
    </div>
  )
}
Repository: h2oai/wave
File: ui/src/menu_table_cell_type.tsx (TypeScript)

Code before:
  /** Items to render. */
  commands: Command[]
 /** An identifying name for this component. */
  name?: S
 }

const css = stylesheet({
  container: {
    display: 'inline-flex',
    alignItems: 'center',
    cursor: 'pointer',
  },
   icon: {
    fontSize: 18
  }
})

export const XMenuTableCellType = ({ model }: { model: MenuTableCellType & { rowId: string } }) => {
  const
    ref = React.useRef<HTMLDivElement>(null),
    [isMenuHidden, setIsMenuHidden] = React.useState(true),
    open = () => setIsMenuHidden(false),
    close = () => setIsMenuHidden(true)

  return (
    <div data-test={model.name} className={clas(css.container, 'w-menu')} ref={ref} onClick={open}>
      <Fluent.FontIcon className={css.icon} iconName="MoreVertical" />
      <Fluent.ContextualMenu
        items={toCommands(model.commands.map(i => ({...i, value: model.rowId })))}
        target={ref}
        hidden={isMenuHidden}
        onDismiss={close}
        isBeakVisible
        directionalHint={Fluent.DirectionalHint.bottomRightEdge}
        calloutProps={{ styles: { beak: { border: border(1, cssVar('$neutralQuaternaryAlt')) } } }}
        styles={{ list: { border: border(1, cssVar('$neutralQuaternaryAlt')) } }}
      />
    </div>
  )
}

Review comment (suggestion):
No need for `w-menu` selector - it's only used in cards that have background color controllable - header/nav - so that we can select these components and invert their colors accordingly.

Code after:
  /** Items to render. */
  commands: Command[]
 /** An identifying name for this component. */
  name?: S
 }

const css = stylesheet({
  container: {
    display: 'inline',
    cursor: 'pointer',
  },
  icon: {
    fontSize: 18
  }
})

export const XMenuTableCellType = ({ model }: { model: MenuTableCellType & { rowId: string } }) => {
  const
    ref = React.useRef<HTMLDivElement>(null),
    [isMenuHidden, setIsMenuHidden] = React.useState(true),
    open = () => setIsMenuHidden(false),
    close = () => setIsMenuHidden(true)

  return (
    <div data-test={model.name} className={css.container} ref={ref} onClick={open}>
      <Fluent.FontIcon className={css.icon} iconName="MoreVertical" />
      <Fluent.ContextualMenu
        items={toCommands(model.commands.map(i => ({...i, value: model.rowId })))}
        target={ref}
        hidden={isMenuHidden}
        onDismiss={close}
        isBeakVisible
        directionalHint={Fluent.DirectionalHint.bottomCenter}
        calloutProps={{ styles: { beak: { border: border(1, cssVar('$neutralQuaternaryAlt')) } } }}
        styles={{ list: { border: border(1, cssVar('$neutralQuaternaryAlt')) } }}
      />
    </div>
  )
}
Repository: h2oai/wave
File: ui/src/combobox.tsx (TypeScript)

Code before:
        if (option && isMultiValued) {
          setSelected(items => {
            const result = selected ? [...items, option as Fluent.IComboBoxOption] : items.filter(item => item.key !== option!.key)
            wave.args[m.name] = result.map(item => item!.text)
            return result
          })
        } else {
          wave.args[m.name] = v
          setText(v)
        }
        
        if (m.trigger) wave.push()
      }
    // eslint-disable-next-line react-hooks/exhaustive-deps
    React.useEffect(() => { wave.args[m.name] = m?.value || m?.values || null }, [])

    return (
      <Fluent.ComboBox
        data-test={m.name}
        label={m.label}
        placeholder={m.placeholder}
        options={options}
        multiSelect={isMultiValued}
        selectedKey={isMultiValued ? selected.map(s => s.key as S) : undefined}
        text={isMultiValued ? undefined : text}
        allowFreeform={allowFreeform}
        disabled={m.disabled}
        autoComplete="on"
        errorMessage={m.error}
        onChange={onChange}
      />
    )
  }

Review comment (suggestion):
Should be enough since it is always true
```suggestion
        allowFreeform
```

Code after:
        else {
          const v = option?.text || value || ''
          wave.args[m.name] = v
          setText(v)
        }
        if (m.trigger) wave.push()
      }
    // eslint-disable-next-line react-hooks/exhaustive-deps
    React.useEffect(() => { wave.args[m.name] = m?.value || m?.values || null }, [])

    return (
      <Fluent.ComboBox
        data-test={m.name}
        label={m.label}
        placeholder={m.placeholder}
        options={options}
        required={m.required}
        multiSelect={isMultiValued}
        selectedKey={isMultiValued ? selected.map(s => s.key as S) : undefined}
        text={isMultiValued ? undefined : text}
        allowFreeform
        disabled={m.disabled}
        autoComplete="on"
        errorMessage={m.error}
        onChange={onChange}
      />
    )
  }
Repository: h2oai/wave
File: ui/src/combobox.tsx (TypeScript)

Code before:
          setSelected(items => {
            const result = selected ? [...items, option as Fluent.IComboBoxOption] : items.filter(item => item.key !== option!.key)
            wave.args[m.name] = result.map(item => item!.text)
            return result
          })
        } else {
          wave.args[m.name] = v
          setText(v)
        }
        
        if (m.trigger) wave.push()
      }
    // eslint-disable-next-line react-hooks/exhaustive-deps
    React.useEffect(() => { wave.args[m.name] = m?.value || m?.values || null }, [])

    return (
      <Fluent.ComboBox
        data-test={m.name}
        label={m.label}
        placeholder={m.placeholder}
        options={options}
        required={m.required}
        multiSelect={isMultiValued}
        selectedKey={isMultiValued ? selected.map(s => s.key as S) : undefined}
        text={isMultiValued ? undefined : text}
        allowFreeform={true}
        disabled={m.disabled}
        autoComplete="on"
        errorMessage={m.error}
        onChange={onChange}
      />
    )
  }

Review comment (suggestion):
nit
```suggestion
        allowFreeform
```
```suggestion
        allowFreeform={true}
```

Code after:
        else {
          const v = option?.text || value || ''
          wave.args[m.name] = v
          setText(v)
        }
        if (m.trigger) wave.push()
      }
    // eslint-disable-next-line react-hooks/exhaustive-deps
    React.useEffect(() => { wave.args[m.name] = m?.value || m?.values || null }, [])

    return (
      <Fluent.ComboBox
        data-test={m.name}
        label={m.label}
        placeholder={m.placeholder}
        options={options}
        required={m.required}
        multiSelect={isMultiValued}
        selectedKey={isMultiValued ? selected.map(s => s.key as S) : undefined}
        text={isMultiValued ? undefined : text}
        allowFreeform
        disabled={m.disabled}
        autoComplete="on"
        errorMessage={m.error}
        onChange={onChange}
      />
    )
  }
Repository: h2oai/wave
File: ui/src/text_annotator.tsx (TypeScript)

Code before:

/** Create an annotator item with initial selected tags or no tag for plaintext. */
interface TextAnnotatorItem {
  /** Text to be highlighted. */
  text: S
  /** Tag connected to the highlighted text. */
  tag?: S
}

/**
 * Create a text annotator component.
 * 
 * The text annotator component enables user to manually annotate parts of text. Useful for NLP data prep.
 */
export interface TextAnnotator {
  /** An identifying name for this component. */
  name: Id
  /** The text annotator's title. */
  title: S
  /** List of tags the user can annotate with. */
  tags: TextAnnotatorTag[]
  /** Pretagged parts of text content. */
  items: TextAnnotatorItem[]
  /** True if the form should be submitted when the annotator value changes. */
  trigger?: B
  /** True to prevent user interaction with the annotator component. */
  readonly?: B
}

const css = stylesheet({
  title: {
    color: cssVar('$neutralPrimary'),
    marginBottom: 8
  },
  tags: {
    display: 'flex',
    flexWrap: 'wrap',
    marginTop: 1
  },
  tag: {
    padding: padding(4, 16),
    textAlign: 'center',
    borderRadius: 4,
  },
  tagWrapper: {
    marginRight: 4,
    marginBottom: 4,
    border: border(2, cssVar('$card')),
    padding: 1
  },
  mark: {

Review comment (suggestion):
```suggestion
  /** True to prevent user interaction with the annotator component. Defaults to False. */
```

Code after:

/** Create an annotator item with initial selected tags or no tag for plaintext. */
interface TextAnnotatorItem {
  /** Text to be highlighted. */
  text: S
  /** Tag connected to the highlighted text. */
  tag?: S
}

/**
 * Create a text annotator component.
 * 
 * The text annotator component enables user to manually annotate parts of text. Useful for NLP data prep.
 */
export interface TextAnnotator {
  /** An identifying name for this component. */
  name: Id
  /** The text annotator's title. */
  title: S
  /** List of tags the user can annotate with. */
  tags: TextAnnotatorTag[]
  /** Pretagged parts of text content. */
  items: TextAnnotatorItem[]
  /** True if the form should be submitted when the annotator value changes. */
  trigger?: B
  /** True to prevent user interaction with the annotator component. Defaults to False. */
  readonly?: B
}

const css = stylesheet({
  title: {
    color: cssVar('$neutralPrimary'),
    marginBottom: 8
  },
  tags: {
    display: 'flex',
    flexWrap: 'wrap',
    marginTop: 1
  },
  tag: {
    padding: padding(4, 16),
    textAlign: 'center',
    borderRadius: 4,
  },
  tagWrapper: {
    marginRight: 4,
    marginBottom: 4,
    border: border(2, cssVar('$card')),
    padding: 1
  },
  mark: {
Repository: h2oai/wave
File: ui/src/stat_table.tsx (TypeScript)

Code before:

/**
 * Render a card displaying a table of stats.
 */
interface State {
  /** The title. */
  title: S
  /** The names of this table's columns. */
  columns: S[]
  /** The rows displayed in this table. */
  items: StatTableItem[]
  // TODO optional data for buffer-based rendering.
  /** An optional name for this item. */
  name?: S
  /** The subtitle, displayed below the title. */
  subtitle?: S
}

/** Create a stat item (a label and a set of values) for stat_table_card. */
export interface StatTableItem {
  /** The label for the row. */
  label: S
  /** The values displayed in the row. */
  values: S[]
  /** List of font colors used for each value in values ordered respectively. Defaults to theme text color if not specified. */
  value_colors?: S[]
  /** An optional name for this row (required only if this row is clickable). */
  name?: S
  /** The caption for the metric, displayed below the label. */
  caption?: S
  /** An optional icon, displayed next to the label. */
  icon?: S
  /** The color of the icon. */
  icon_color?: S
}

const
  css = stylesheet({
    card: {
      display: 'flex',
      flexDirection: 'column',
      padding: 15,
      // HACK: for some reason, the overflow on 'items' does not take effect without this line.
      overflow: 'auto',
    },
    items: {
      flexGrow: 1,
      overflow: 'auto',
      marginTop: '1em',
    },
    table: {

Review comment (suggestion):
Always put new optional attributes at the end since some people may use positional args (not keyword args) in python. 

```suggestion
  /** List of colors used for each value in values ordered respectively. */
  colors?: S[]
```

Code after:

/**
 * Render a card displaying a table of stats.
 */
interface State {
  /** The title. */
  title: S
  /** The names of this table's columns. */
  columns: S[]
  /** The rows displayed in this table. */
  items: StatTableItem[]
  // TODO optional data for buffer-based rendering.
  /** An optional name for this item. */
  name?: S
  /** The subtitle, displayed below the title. */
  subtitle?: S
}

/** Create a stat item (a label and a set of values) for stat_table_card. */
export interface StatTableItem {
  /** The label for the row. */
  label: S
  /** The values displayed in the row. */
  values: S[]
  /** An optional name for this row (required only if this row is clickable). */
  name?: S
  /** The caption for the metric, displayed below the label. */
  caption?: S
  /** An optional icon, displayed next to the label. */
  icon?: S
  /** The color of the icon. */
  icon_color?: S
  /** List of colors used for each value in values ordered respectively. */
  colors?: S[]
}

const
  css = stylesheet({
    card: {
      display: 'flex',
      flexDirection: 'column',
      padding: 15,
      // HACK: for some reason, the overflow on 'items' does not take effect without this line.
      overflow: 'auto',
    },
    items: {
      flexGrow: 1,
      overflow: 'auto',
      marginTop: '1em',
    },
    table: {