noob-components/.trellis/workspace/hechang27-sprt/04-10-code-review.md

 # Code review

  Found 4 urgent issues need to be fixed:

  ## 1 Dimension props regressed, so height/maxHeight no longer follow the old contract

  FilePath: packages/base/data/list-table-v2/list-table-v2.vue:291 line 291

  if (props.height !== undefined) {
    const h = String(props.height);
    style.maxHeight = h.endsWith("px") ? h : `${h}px`;
  }
  // props.maxHeight handling is commented out

  height is being applied as maxHeight, maxHeight is ignored entirely, and minHeight is unused. The old component and the
  demo page both rely on these props behaving distinctly, so PR2 currently breaks the documented sizing behavior.

  ### Suggested fix

  Restore the old container contract: set style.height from props.height, apply style.maxHeight from props.maxHeight, honor
  props.minHeight, and keep viewportHeight derived from the actual constrained container size.

  ———

  ## 2 rowKey is exposed but never used, so virtual rows are keyed by visible index

  FilePath: packages/base/data/list-table-v2/list-table-v2.vue:32 line 32

  <div
    v-for="row in visibleRows"
    :key="row.index"
    class="table-row"

  This throws away stable row identity. On page changes, filtering, or reordered data, Vue will reuse row subtrees by index
  instead of by record identity, which is exactly what rowKey is supposed to prevent.

  ### Suggested fix

  Derive a stable key from the actual row, for example getRow(row.index)?.[props.rowKey] ?? row.index, and use that for the
  row wrapper.

  ———

  ## 3 Pretext width calculation is measuring different text/fonts than the component actually renders

  FilePath: packages/base/data/list-table-v2/usePretextColumnWidths.ts:198 line 198

  const font = options?.font ?? DEFAULT_FONT;
  const headerFont = options?.headerFont ?? DEFAULT_HEADER_FONT;
  ...
  const headerText = col.name || col.i18n || colKey;

  The component renders body text with props.font and now defaults to 14px Microsoft YaHei, but list-table-v2.vue still
  calls this hook with hardcoded Inter fonts. On top of that, header sizing uses the raw i18n key instead of the translated
  label. That means the measured widths diverge from the rendered widths, which directly undermines PR2’s “column widths
  adapt to content” acceptance criterion.

  ### Suggested fix

  Pass the real rendered font into usePretextColumnWidths from packages/base/data/list-table-v2/list-table-v2.vue:241,
  derive the header font from that same base font, and measure the translated header text instead of col.i18n.

  ———

  ## 4 The rewrite dropped timestampFormat from the public API

  FilePath: packages/base/data/list-table-v2/list-table-v2.vue:110 line 110

  interface Props {
    ...
    border?: boolean;
    rowHeight?: number;
    estimatedRowHeight?: number;
    headerHeight?: number;
    font?: string;
    debug?: boolean;
  }

  The old component exposed timestampFormat, but the new runtime props and exported ListTableProps no longer include it,
  while timestamp rendering is still part of the component contract. That is a backward-compatibility break against the PRD
  goal of keeping the existing prop interface.

  ### Suggested fix

  Re-add timestampFormat to both the runtime props and packages/base/data/list-table-v2/types.ts:87, then thread it into the
  timestamp formatting path so existing consumers keep their display override.

  ———

  Would you like me to use the Suggested fix section to address these issues?


# Response
› 4 is intentional, 1/2/3 should be fixed. However, there is an additional problem: when using custom slots like the table
  11 in the demo page, the inner elements consistently overflows the table cell even with overflow: hidden...