Skip to content

feat(dq): render the AI-mode form hint as an in-modal column#30195

Draft
ShaileshParmar11 wants to merge 17 commits into
mainfrom
feat/dq-ai-form-hint-panel
Draft

feat(dq): render the AI-mode form hint as an in-modal column#30195
ShaileshParmar11 wants to merge 17 commits into
mainfrom
feat/dq-ai-form-hint-panel

Conversation

@ShaileshParmar11

Copy link
Copy Markdown
Contributor

What

Replaces the AI-mode Data Quality form hint — a popover anchored to the focused field, floating outside the modal — with a column inside the modal. Addresses the DQ UI feedback thread (Teddy / Harsha / Karthick).

The popover was the root of every reported symptom: it anchored to whatever field had focus (so it drifted), sat 56px off the modal (the "gap"), and was a genuinely separate layer ("feels broken that both are split"). The old AiFormModal papered over this with a sideways translate and a 22-frame requestAnimationFrame resize pump — both deleted here.

Changes

  • FieldDocPanel (new, core-components) — renders the focused field's doc as a static column; the counterpart to FieldDocPopover, same registry, different presentation.
  • HookForm — new fieldDocDisplay="panel" mode laying out form + hint as two independently-scrolling columns. Popover path (incl. #30143's fieldDocMaxHeight) unchanged.
  • AiFormModal — animates its own width between hint-shown/hidden, stays centered; drops the translate + resize pump. Also extended with optional props (icon/iconColor, footerActions, submitFormId, isSubmitDisabled) so it can be reused by the Collate alert modal.
  • TestCaseFormDrawer / TestDefinitionForm — wired to panel mode; docs typography (12.5px/1.55, Geist Mono for param names) matches the design; parameter names rendered mono via a targeted transform.

Verified live (Collate AI mode + Safari)

Measured against the approved mock: docs body 12.5px/19.375px/rgb(83,88,98), title 16px/600, param names Geist Mono. Column scrolling, focus-following, keep-last-doc, smooth toggle, empty state — all confirmed. Six layout bugs invisible to Jest (jsdom has no layout engine) were found and fixed by driving the real app.

Two deliberate divergences from the mock — please review

  1. Modal is 1152px, not the mock's 1024px. The mock's 642px form column assumed test-level cards that don't wrap; the real cards need ~772px. Widths are now derived from the columns.
  2. Narrow screens shrink the hint, not the form. The mock crushes the form to 267px and clips it around a 713px viewport (where the bug was originally reported, on Safari); this holds the form's floor and yields the hint.

Not in scope (filed separately)

  • The mock's mono param names come with STRING/Required type chips — that needs structured TestDefinition.parameterDefinition data, a feature not a font change.
  • A pre-existing RichTextEditorPreviewerV1 bug renders >/< literally in docs (root cause proven; fix is an XSS-sensitive pipeline change touching 48 surfaces).

🤖 Generated with Claude Code

ShaileshParmar11 and others added 17 commits July 17, 2026 13:34
Renders the focused field's documentation as a static column, as the
counterpart to FieldDocPopover: same source (the FieldDoc registry), different
presentation. Takes no anchor and does no positioning, so it cannot drift from
the surface or track the focused field's vertical position.

FieldDocPopover is unchanged and still exported.

The story is a design-review demo, not a test: this package has no test runner
and tsconfig excludes *.stories.tsx, so play functions never execute in CI.
Real coverage comes from the app-side drawer tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fieldDocDisplay="panel" lays the form and its field docs out as two
independently scrolling columns inside one surface, instead of floating the
doc next to the focused field.

The panel is rendered here rather than by the caller because it reads the
active field from FieldDocProvider, which lives in this component: rendered as
a sibling of the form it would fall outside the provider and always be empty.

The form column carries a min-width floor so the hint yields on narrow
viewports. Without it the hint's width is reserved first and the form absorbs
all the shrink, which crushes the form and clips its content.

The popover path, including #30143's fieldDocMaxHeight, is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ize pump

The Form Hint is becoming a column inside the modal rather than a popover
floating beside it, so the machinery that made a detached layer look attached
is no longer needed:

- the xl-gated translate that slid the modal sideways to fake a centred
  modal+hint group, and
- a 22-frame requestAnimationFrame loop dispatching synthetic window resize
  events, which existed only because react-aria re-anchors popovers on resize
  and not while an ancestor transform animates.

The modal now animates its own width between the hint-shown and hint-hidden
sizes and stays centred in both. hintOpen is tri-state so forms with no hint
column (BundleSuiteFormDrawer) keep the previous single-column layout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The hint was a popover anchored to the focused field and floating outside the
modal, which is why it read as a second, detached box and appeared to drift:
its position tracked whichever field had focus. It is now a column inside the
modal, so it cannot drift and cannot be misaligned with the form.

Focus behaviour is unchanged: clicking a field still shows that field's doc.
Before any field is focused the column shows an EmptyPlaceholder rather than
sitting blank.

Docs typography matches the design: 12.5px/1.55 body with Geist Mono for
inline code. Only `code` is mono, not `strong` — the docs use bold for prose
emphasis as well as parameter names, so styling `strong` would turn prose into
code.

The empty-state string is translated in all 20 locales rather than left as an
English placeholder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…al column

Applies the same treatment as the Add Test Case modal so the AI experience has
one hint behaviour rather than two.

The hint docs typography moves to styles/components/form-hint-doc.less now
that two features share it; a component reaching into another feature's
stylesheet would have been fragile.

BundleSuiteFormDrawer passes no hint props, so hintOpen is undefined and it
keeps the previous single-column layout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Found by driving the real Collate AI modal; unit tests cannot catch it because
jsdom has no layout engine.

The row carried `h-full`. The host caps the body with `max-height` and no
explicit height, so `height: 100%` was an indefinite percentage: it resolved to
`auto` (the full 2174px content height) and suppressed flex `stretch`. The
columns therefore never became scrollable and the host clipped them instead —
the form was cut off at 640px with no scrollbar and no way to reach the fields
below. Letting the row stretch to the clamped height fixes it.

The form's min-width floor also moves to the caller. 380px is a reasonable
default, but the Add Test Case form's test-level cards need ~574px before they
clip, so it raises the floor to 600px; a lower floor let the hint take room
that form could not spare.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Moving the hint inside the modal cost the form width: the AI modal was 820px
with the hint floating outside, so the form had ~772px. At 1024px minus a
380px hint it dropped to 644px, which squeezed the three test-level cards to
167px each. With the icon beside the text that left the label ~63px, so
"Table Level" wrapped mid-phrase.

Both halves of that are addressed:

- SelectionCardGroup gains an opt-in `layout="vertical"` that stacks the icon
  above the text, giving the label the card's full width — the arrangement the
  approved design uses. The default stays horizontal, so the scheduling cards
  that share this component are untouched.
- The modal's widths are now derived from the form and hint column widths
  rather than hardcoded, and the form keeps the 772px it had before. The form
  column is therefore the same width whether the hint is shown or hidden; only
  the hint appears and disappears.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The header is a justify-between row, so with the hint collapsed the actions
were compressed and "Show Hint" wrapped onto two lines.

The actions no longer shrink, and the title block absorbs the squeeze instead
(its subtitle already wraps). Fixed in AiFormModal so every consumer benefits,
with nowrap on the labels themselves as the belt-and-braces.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…field

The card group sat flush against its label while every other field on the form
leaves 6px, which made the section read as though the label belonged to the
cards' container rather than to the field.

6px matches the shared field wrapper's gap; there is no LESS variable for it
(@size-xxs is 4px, @size-xs is 8px), so it is spelled out with a pointer to
where the number comes from.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous rule set 12.5px on the wrapper and looked correct, but the docs
still rendered at 14px/20px in near-black: RichTextEditorPreviewerV1's TipTap
block editor declares its own size on
`.block-editor-wrapper .tiptap.ProseMirror p`, which out-specifies anything the
wrapper inherits down. The selectors now mirror that chain so they win, and the
doc prose picks up the secondary text colour the design uses.

The panel title was medium where the design is semibold.

Measured against the design in the running app: body 12.5px/19.375px in
rgb(83,88,98) and title 16px/600 now match exactly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… does

The design renders a parameter's name in mono and its description in Inter. Our
docs mark parameter names with bold, but they use the same
`- **lead-in**: prose` shape for descriptive bullets, so styling every leading
bold would put prose into monospace.

Parameter bullets are distinguishable by their type parenthetical
("- **Min** (INT) - ..."), which descriptive bullets never carry. Only those are
rewritten to code, so they pick up the panel's existing mono styling. Against
the real docs that is 55 parameters converted and 37 descriptive bullets left
alone, with no false positives.

Length is not a usable signal and was tried first: the longest name in the docs
is a real parameter and several short ones are prose.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The modal was not animating at all: Dialog puts `className` on the outer dialog
wrapper, but applies `width` as max-width on an inner panel, so the transition
class never reached the element it was meant to animate. Measured: the panel's
transition-duration was 0s, the modal jumped to its new width within one frame
while the hint column animated over 240ms, and for 26 frames the hint was wider
than the modal containing it — visibly overflowing and clipped mid-toggle.

Dialog gains an opt-in `panelClassName` for the element `width` lands on. Other
dialogs are unaffected.

Both now share the same duration and easing, and the form column is a fixed
width, so modal width and hint width stay in lockstep for the whole transition.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The registry clears the active field on blur, which is right for the popover —
it closes. The panel is always on screen, so the same behaviour blanked it back
to the empty state whenever focus landed on an undocumented control (the
description editor, a button, empty space), wiping the hint out mid-task.

The panel now holds the last doc it showed. This is a presentation choice, so
it lives in the panel rather than the shared registry, and the popover is
unaffected.

It forgets while docs are switched off: the panel stays mounted (collapsed to
zero width) so toggling never remounts the form, so a remembered doc would
otherwise linger in the DOM while hidden and reappear in place of the empty
state when switched back on.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dialog's header and footer are built for a padded single-column dialog, which
does not suit an edge-to-edge two-column body:

- Dialog.Footer ships `mt-6 sm:mt-8`, leaving 32px of white between the columns
  and the footer, so the hint column visibly stopped short of it. The columns
  already reach the modal's edges, so the footer now sits against them.
- Dialog.Header ships no bottom padding, so the body sat flush against the
  title.

The body's height cap accounts for the taller header: at `88vh - 152px` the
modal came to 797px against its own 792px cap. Subtracting the real chrome
height (157px) brings it to 752px.

The cap is spelled out rather than interpolated from a constant — Tailwind
scans source statically, so an interpolated arbitrary value generates no class
and the cap would silently disappear.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
20px left too large a band under the title. 4px is enough to separate the
header from the body, which supplies the rest of the spacing with its own top
padding. The height cap follows the now-shorter header: measured chrome is
141px (72 header + 69 footer), so the body cap is calc(88vh-141px).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds five optional props so a second family of AI modals (the alert add/edit
modal) can use this shell instead of hand-rolling its own copy:

- icon / iconColor — vary the header featured-icon (alert uses AlertTriangle
  in a warning tone); defaults keep the DQ gray CheckCircle.
- footerActions — extra buttons between Cancel and Submit (the alert's dynamic
  test/extra buttons).
- submitFormId — when set, the submit button submits a `<HookForm id=...>`
  natively instead of calling onSubmit; onSubmit is now optional for that path.
- isSubmitDisabled — disable submit while the form loads.

All optional; the three existing consumers are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Jul 17, 2026
* The trailing `[,)]` matters: an earlier version required a comma and silently
* skipped the 20 `(INT)` parameters, leaving them bold amongst mono siblings.
*/
const PARAMETER_BULLET = /^(\s*[-*]\s+)\*\*([^*]+)\*\*(\s*\([A-Z]+[,)])/gm;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Edge Case: Param-name mono transform silently skips non-A-Z type tokens

PARAMETER_BULLET in FormHintDocUtils.ts requires the type parenthetical to be strictly [A-Z]+ immediately followed by , or ). All type tokens currently in TestCaseForm.md (INT, NUMBER, STRING, FLOAT, BOOLEAN, ARRAY, etc.) match, so this works today. But any future/edited parameter whose type contains digits, lowercase letters, spaces, or angle brackets (e.g. INT64, ARRAY<STRING>, Boolean) will silently fail to match and render bold instead of mono, with no error — inconsistent styling that is easy to miss. Consider broadening the character class (e.g. [A-Za-z0-9<>_ ]+) if such types are anticipated, or add a note that types must stay all-caps.

Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Refactors the AI-mode form hint into a static modal column to resolve UI drift and layering issues. While this implementation matches the design, the parameter-name mono transform in FormHintDocUtils should be updated to include non-A-Z tokens.

💡 Edge Case: Param-name mono transform silently skips non-A-Z type tokens

📄 openmetadata-ui/src/main/resources/ui/src/utils/DataQuality/FormHintDocUtils.ts:32

PARAMETER_BULLET in FormHintDocUtils.ts requires the type parenthetical to be strictly [A-Z]+ immediately followed by , or ). All type tokens currently in TestCaseForm.md (INT, NUMBER, STRING, FLOAT, BOOLEAN, ARRAY, etc.) match, so this works today. But any future/edited parameter whose type contains digits, lowercase letters, spaces, or angle brackets (e.g. INT64, ARRAY<STRING>, Boolean) will silently fail to match and render bold instead of mono, with no error — inconsistent styling that is easy to miss. Consider broadening the character class (e.g. [A-Za-z0-9<>_ ]+) if such types are anticipated, or add a note that types must stay all-caps.

🤖 Prompt for agents
Code Review: Refactors the AI-mode form hint into a static modal column to resolve UI drift and layering issues. While this implementation matches the design, the parameter-name mono transform in `FormHintDocUtils` should be updated to include non-A-Z tokens.

1. 💡 Edge Case: Param-name mono transform silently skips non-A-Z type tokens
   Files: openmetadata-ui/src/main/resources/ui/src/utils/DataQuality/FormHintDocUtils.ts:32

   `PARAMETER_BULLET` in FormHintDocUtils.ts requires the type parenthetical to be strictly `[A-Z]+` immediately followed by `,` or `)`. All type tokens currently in TestCaseForm.md (INT, NUMBER, STRING, FLOAT, BOOLEAN, ARRAY, etc.) match, so this works today. But any future/edited parameter whose type contains digits, lowercase letters, spaces, or angle brackets (e.g. `INT64`, `ARRAY<STRING>`, `Boolean`) will silently fail to match and render bold instead of mono, with no error — inconsistent styling that is easy to miss. Consider broadening the character class (e.g. `[A-Za-z0-9<>_ ]+`) if such types are anticipated, or add a note that types must stay all-caps.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant