feat(fds): adopt the library Combobox and Select across the product selection fields — DO NOT MERGE, pending design review - #7565
Open
Sandy Ghs (sandyghs) wants to merge 61 commits into
Conversation
Contributor
|
📖 Documentation check — ✅ Passed 102 functional file(s), 0 doc file(s) changed. No documentation gaps detected. |
Filigran Automation (Filigran-Automation)
added
the
filigran team
Contributor
|
Core CI reports these findings in advisory mode. Review the JSON reports before merging.
View workflow run · Standard JSON report · UBI9 JSON report Updated from CI run attempt 1. |
Sandy Ghs (sandyghs)
changed the title
Sandy Ghs (sandyghs)
changed the title
Member
|
Tests:
- Action creation
Expectations fields are overlapping on label/placeholder
- Action creation
Weird spacing on domain field
- Filters
Filters are bigger now
Tag filter has a big space
Dropdown list is smaller
- Scenario creation is not aligned
Before
Now
- Scenario with chaining in Attack Path tab is not aligned
- Widget creation
Label in dropdown are smaller
- Profil dropdown
Dropdown is large
|
Sandy Ghs (sandyghs)
changed the title
…token bridge The Combobox fixes land with this pin: the field derives its text from the selection in single mode, selection-on-focus, opening-on-focus as a prop, focus retained on an option click, and the active row scrolled into view. The pin also carries the disabled-token wave. Two tokens this product wires into its MUI theme change value, so the effect is product-wide, not scoped to the Combobox: --text-default-disabled dark #a0b4e3 -> #95969d light #2b4f8d -> #62636a --bg-elevation-disabled dark #0c1527 -> #313235 light #c8d6ee -> #cacbce palette.text.disabled has 74 references in this front. The shift is from a blue-tinted to a neutral gray ramp; disabled controls are exempt from WCAG 1.4.3, so this is a legibility change, not a conformance one. --color-filigran-brand-primary-transparency-50 was renamed to -55. No product code reads it, only the generated bridge, so the rename is inert here.
…gex field and the platform field
… security-platform fields
Both fields were uncontrolled MUI Autocompletes. The library Combobox is always controlled, so each now holds its own transient selection in local state — the state the uncontrolled MUI kept internally. Both read MUI's `clear` change reason, which has no library equivalent; measured at the only call site, the clear and change handlers reduce to the same state update, so an empty selection takes the clear path.
…igger fields Both dropped a custom chip renderer that only carried presentation (size small); the field now holds the library Chip row.
…picker The file stays mixed on purpose: the search field beside it keeps a leading magnifier, which the library has no slot for. It is aliased and marked. The product's slotProps override that widened the MUI dropdown beyond the closed field is dropped: the library panel already sizes itself min-w = trigger width, max-w-82.
Both fields were uncontrolled, so each now holds its own selection in local state. The mapper field drops MUI's createFilterOptions for the library's own filterOptions, which performs the same case-insensitive contains, and its hint rows move from pointer-events: none to isOptionDisabled, so they are no longer reachable by keyboard. The two field labels now go through the translator like every other label in these files; they render the same text.
The i18n gate runs before the unit tests and went red on three keys this wave introduced: Sheet, Mapper and Loading, missing in all nine locales. Sheet and Mapper were already untranslated before the wave (label="Sheet" on a MUI TextField); they stay untranslated rather than receive nine invented translations. Loading is dropped and the library's own default loading message is used.
…ets land The three native-reset defects this wave measured are fixed upstream. Re-measured in this product's environment, both modes: the single-row field, the filter and the chip field now all render 36px, the value Combobox.figma.json declares, down from 37/37/63. No token moved: theme.css carries the same hash at both pins, so the regenerated bridge is byte-identical.
The pin brings both library fixes: #149 makes the Combobox panel read --fds-z-overlay, and #150 lands the Figma alignment plus five behaviours the wave was blocked on — a change cause on every callback, closeOnSelect, keepInputOnBlur, allowCustomValue and creation on the fly. The host raises its overlay layer from 1300 to 1400. Measured against this app's filter Popover at zIndex.modal 1300: at 50, 1200 and 1299 the MUI backdrop owns the pixel at an option row; at 1300 and 1400 the row does. 1300 works, but only because the library's portal is appended after MUI's, so the tie resolves in its favour. 1400 does not depend on that insertion order. No token moved: theme.css carries the same hash at both pins and the regenerated bridge is byte-identical.
First site unblocked by #150's change cause: the field protected its typed text from a programmatic reset with MUI's `reason === 'reset'`; the library states the same cause, so the guard becomes `meta.cause !== 'type'`. Reaches five mount sites downstream.
Three sites #150 unblocked with allowCustomValue plus createValueFromInput: the cloud native type field, the separator field whose options are objects, and the filter chip input, which is a free-text token entry with no option list. FilterChipPopoverInput stays mixed on purpose: its second site drives selection from its own row handler, which the library's row contract does not allow. It is aliased and marked.
Second site unblocked by the change cause: the field drives a server search and guarded it with MUI's `reason === 'input'`, which becomes `meta.cause === 'type'`. Reaches three mount sites downstream.
Re-pin on cd4b8ee (library #155: controlled `open`, `startIcon`, `adornment`). Converted here: - 7 controlled-open sites: the `AutocompleteField` wrapper and its 5 typed callers, `ScenarioField` (reads the close cause via `meta.cause`), and the 5 chips-only token inputs on `open={false}` with no `onOpenChange`. - 1 row-interaction site: `FilterChipPopoverInput` diffs the new selection instead of owning the option row's click. - 1 leading-icon site: the attack-path search on `startIcon`. - 4 ornament sites: the create button moves to `adornment`, dropping the absolute positioning that dodged MUI's own controls. Also: the `renderOption` prop of `AutocompleteField` only ever tested for `null`, so it becomes `hideOption`; three identity `renderOption` are removed, which restores the 32px option row the library gives without one. 41 of 44 sites now on the library. The 3 remaining are ToolBar, settled.
…ng keys Three faults found by CI, all introduced by the previous commit: - The ornament buttons were all labelled "Create", so the arsenal creation form exposed three controls with that one name — ambiguous for a screen reader, and what four infra E2E jobs reported as a strict-mode violation. Each now names its own object, reusing the key its creation dialog already carries. - `No result` did not exist in any locale; the field uses `No results found`, which does. - The required-field helper text was invented here: the wrapper never showed a message, only an error state. Removed rather than translated.
Playwright's accessible-name matching is a substring match by default, so `name: 'Create'` also selected the field ornaments now named "Create a new tag" and "Create a new attack pattern". The names are right — three controls called "Create" would be ambiguous — so the locator becomes exact.
The arsenal form assertions reached for `.MuiFormHelperText-root.Mui-error` under a `MuiFormControl-root` ancestor. The platforms field stopped having either when it was converted, so these assertions have been red since that wave — not caught then, and reported green in error. The helper now matches both shapes and only one can resolve per field.
… multiple The library Select holds one string, so the three multi-value platform pickers become Comboboxes — the nearest component that holds a set. Their trigger used to render either the chosen platforms or the sentence "Any security platform"; with chips holding the selection, that sentence becomes the input's placeholder, which shows exactly when nothing is chosen. The fourth is a disabled single field with one fixed row.
…, surfaces - The filter field sat flush against the search box it follows: measured 0px on the list toolbar. The gap goes on that boundary, inside the field's own container, so it reaches every page that mounts it rather than one screen. - Statistics row: the parameter fields and the time range are label-above-input, and the grid was stretching them to the tallest row so their inputs stopped lining up. The simulation field also had no placeholder — it now shows the parameter's own name, which introduces no translation key. - The filter field kept its typed text after adding a filter. Same root cause as the five token inputs: the `cause === 'type'` guard drops the library's own reset. It was missed there because the search for it was scoped to the chips fields and this one holds no chips — its guard even carries a comment explaining why non-type causes were ignored, which is what hid it. - Drawers and the filter popover read their ground from `--bg-elevation-default-layer-2` instead of MUI's paper, through explicit hooks so no other paper in the app moves.
The library Select renders no wrapper of its own — unlike Combobox, which wraps its parts in a flex column. Its label and trigger are therefore siblings of whatever holds them, and in the statistics grid they landed in two different cells: measured, the parameter field, the label and the trigger all sat at the same top. That is the misalignment the review reported. Wrapped here, and recorded as LIBRARY-FEEDBACK 43: the component should provide the wrapper, since every site otherwise has to know this.
…sterisk The library marks a required field with an `aria-hidden` asterisk, so it is deliberately absent from the accessible name; MUI used to fold the marker into the label's own text. Four locators asked for "Type *", "Expectations *", "Architecture *" and "Executor *" and timed out on the infra suites. Matched on the label alone, exact, so the shorter name cannot collide.
…y the asterisk Two fields on the action form share the label "Executor"; the "*" in the old locator was what told them apart. The library hides that marker from the accessible name, so the four locators now narrow on `aria-required` — which is what the asterisk stood for.
`aria-required` was applied to all four required-looking fields, but only Executor has two same-labelled siblings and only it carries the attribute — the other three then matched nothing. The narrowing stays where the collision was measured; the rest match on the exact label alone.
The two field families expose a required marker differently: `SelectLabel` takes `required` and renders it `aria-hidden`, while `ComboboxLabel` has no such prop, so sites append it as text and it stays in the name. "Expectations" is a Combobox, so its name is "Expectations *"; "Type" is a Select, so its name is "Type". Recorded as LIBRARY-FEEDBACK 44 — the asymmetry is what broke four locators in three different ways.
Consequence of LIBRARY-FEEDBACK 43: the library Select renders no wrapper, so its helper text is a plain sibling of the trigger rather than a child of a root. The helper now covers all three shapes present while the migration is partial — MUI FormControl, Combobox root, Select sibling.
Select derives its helper-text id from a bare `React.useId()`, while Combobox
builds `${baseId}-helper`. The suffix predicate therefore never matched a
Select, and the sibling — the only paragraph beside the trigger — is matched
directly instead.
…hs, tag reset **Filter bar.** The search box, the filter field and the clear button now sit on one axis 8px apart. Measured before: 0px to the field and 20px to the button, heights 38/36/36. After: 8/8, one top, one bottom, three 36. Two real causes — the search box was 38 high, and `FilterAutocomplete` spread its `style` onto the clear button, where it leaked a margin. `style` now applies to the row, and the gap to whatever precedes the field lives in the one caller that knows there is something to sit beside. The execution toolbar had its own 12px gap plus a 10px margin on the tag field; both go. **Parameter type.** The field shrank to its trigger's content, 100px, while its label needs 107px of text and wrapped onto two lines. 160px now, label on one line. The three form wrappers also restore `fullWidth`: the library trigger sizes to content, so a field was narrower than its own label. **Widget dimension.** The entity type and the filter field were on two rows. They are one row now — 8px apart, clear button on the same axis, chips below. Measured 8/8, one bottom, three 36. **Tag field.** Adding a tag left its name in the field. Two causes, not one: the component kept the tag as its own `value`, and the library holds the input's text separately — clearing the value does not clear that text. Both are cleared. This is why the earlier "reset cause" pass missed it: the text was never leftover typing, it was the field's value. **Filter popover.** It was painted layer-2, the same value as `--bg-input-default`, so it vanished behind its own fields. Figma node 7346:48677 reads layer-3 with `--radius-sm`, `--spacing-4` of padding, `--spacing-2` between fields and the shared shadow — applied. In LIGHT mode layer-3 is also the fields' own value and a library field's border is transparent until focus, so a border keeps the panel readable; no elevation token holds the design's direction in both modes.
…, not ordering The node 7346:48677 is authored in dark, where the panel sits above its fields (#1f3965 over #13213e). The intent is that panel and fields stay DISTINCT and the panel detaches from the page — not that the panel is always the lighter of the two. In light the hierarchy inverts: a very light panel with darker fields on it. So the two modes take different tokens, and that is deliberate: panel: layer-3 dark / layer-1 light, per the design node — separation over absolute ordering. Measured in the real light theme (profile switched, then restored): page #f2f2f3, panel #fff, fields #e4e5e7 — panel lighter than the page, fields darker than the panel, both separations present. Dark: page #0d172b, panel #1f3965, fields #13213e. Two values corrected against the node itself: the panel's padding is `--spacing-2`, not `--spacing-4` — that 16px is the FIELD's own left padding, which the first pass misread as the panel's — and the shadow is the node's `0 2px 2px`. The compensation border is gone: the node shows none, and it is no longer needed now that light has its own token.
…e button **Orphan MUI labels over library fields.** In the action creation drawer the "Prevention" and "Detection" labels sat ON their own field: measured, label at y=667 and field at y=669, against a 26px label→field offset on every other row in the same form. Cause: those two kept a MUI `InputLabel`, which is `position: absolute; top: 0` with a scale transform — a floating label that positions itself INSIDE the control, because a MUI input reserves a notch for it. A library field reserves nothing, so the label lands on the field. The label is now the library's own. An AST sweep found the same pattern on four more fields (ToolBar's action field, both expectation forms) — all five fixed, and all six rows in the drawer now share the same 26px offset. **Chip row alignment.** On the simulation execution toolbar the tag chips sat 3px below the two fields they sit beside: the chip container carried `margin: 5px 0 0 0` and a `float: left` that does nothing to a flex item, both left over from the pre-flex layout. The row centres its items, so measured after: search box, tag field and chip all on centre 545. **Create button moved — a deliberate UX change.** It rode the pagination row, and the card view makes that row wider than the list view because it adds the sort select there. Measured at 1400px wide: 47px of the button visible in list view, 0 in card view, with no horizontal scroll to reach it. It now sits in the page header beside the import icons — one instance, rendered in both views, 8px from the icon group, and the pagination row it leaves is otherwise unchanged. Covered by tests_e2e/tests/threat-arsenals/threatArsenal-list-header.spec.ts.
The toolbar put selection, search, filters, sort, page size and pagination on a single row. Measured at a 1400px viewport before: "1-20 sur 26" ended at x=1450 and the pagination arrows sat at x=1471 and x=1511 — past the window, with no horizontal scroll to reach them. The arrows were unreachable, not just cramped. Three rows now: - what acts on the whole list — the selection controls on the left, page size and pagination on the right; - what narrows it — search, filter field with its clear button, sort; - the filter chips, which already render nothing when there are none. Measured after, both views at 1400px: nothing past the window, no horizontal scroll, the next-page arrow at x=1338, 8px between neighbours inside a row and one axis per row. Selection, filters, sort and pagination all still work — select-all reports "26 actions sélectionnées" and opens the bulk bar, next page moves to "21-26 sur 26", clearing filters removes the chips row. Three spacings that used to compensate for the single row are gone, now that the row itself provides the 8px: the filter field's left margin, the matrix button's, and the sort control's. The chips row drops its own vertical padding for the same reason. `topPagination` is removed: it selected an alternate arrangement that none of the 60 pages mounting this toolbar ever asked for, so the branch was dead. Covered by tests_e2e/tests/threat-arsenals/threatArsenal-toolbar.spec.ts, which fails on the previous layout: neither row anchor exists there and the next-page arrow reports x=1458, outside a 1400px window.
The field does not open its list on focus — clicking the input leaves it closed, so the option never arrived and the chips test timed out. Verified in the browser first: the shell's trigger opens a list of 7 options.
Its accessible name follows the UI language, so a name-based locator only holds in one locale. Verified in the browser: the anchor finds the button and the chips row disappears after the click.
Picking a filter opens its configuration popover over the toolbar, so the clear button underneath was found but never clickable — Playwright reported it waiting for an element to become visible, enabled and stable.
Three of the four review points on the list toolbar. Each cause was measured before it was touched. **Row height 52 -> 40.** MUI wraps `TablePagination` in a Toolbar whose own `minHeight` is 52, and its two captions are paragraphs carrying the browser's 1em vertical margins — 12.8px each side of a 19px line. Zeroing those margins is what brought the row down: with the minHeight alone it still measured 45. The page arrows shrink to sit inside 40 rather than being clipped by it. **Sort label centring.** "Sort by" is the library's `SelectLabel`, which carries `mb-2` because it is designed to sit ABOVE its field. Here it sits beside it, and in a centred flex row a bottom margin lifts the item's visible box — measured 4px above its two neighbours (centre 292 against 296). The margin is cancelled where the label is used horizontally; measured after, all three on 288. **Chips row spacing.** The chips row is a SIBLING of the toolbar, not one of its children, so the toolbar's own 8px row gap never reached it — measured 0. The 8px is declared on the row itself. The fourth point, collapsing the toolbar back to one line on a wide screen, is NOT in this commit. See the report: a first attempt is measured and rejected.
The row asked for 40 but rendered taller: MUI's TablePagination Toolbar carries its own minHeight, and the select-all Checkbox its own padding. Both are fixed where they are set rather than compensated downstream. Measured after the change: top row 36px, checkbox click box 36x36, pagination arrows 32x32 — both above the 24x24 target-size minimum, so the height costs nothing in reachability. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One fixed break point, expressed as a media query. Below it the toolbar keeps the two-row stack; at or above it the row wrapper stops generating a box, so the selection and pagination groups become items of the toolbar itself and share a line with the filters. `order` moves the pagination last, `margin-left: auto` holds it at the far edge. Nothing measures anything at runtime and no state depends on the width, so the layout cannot disagree with the decision that produced it — which is what defeated five earlier adaptive attempts. The groups shrink rather than wrap, which is why the tight case fits: on the arsenal list at 1600px the three groups use 993px of the 1106px the toolbar has. The toolbar is shared by 62 pages, so the one-line container also wraps: a page carrying heavier slots falls back to two rows instead of running past its container. The filter chips are rendered outside the toolbar and keep a line of their own in both arrangements — measured at 8px below either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… line count The 1600px test asserted that the filters sit level with the pagination. That is a consequence of the arrangement, not the arrangement itself, and the one-line container is deliberately allowed to wrap so that a page with heavier slots never overflows. CI measured exactly that: the card view carries the sort select the list view does not, so at 1600px it wraps to two rows (44px offset) while the list view stays on one. Nothing overflowed — the assertion was wrong, not the layout. Both sides of the break point are now read from the wrapper's computed `display`: `contents` above, `flex` below. That is the media query itself, so it holds whatever the available width, locale or slots turn out to be. Also drops two proper names from a CSS comment, which name a design node that already has an identifier. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The journal stopped at "adoption closed at 41 of 44", so it did not record the Select family conversion, the visual review fixes, or the toolbar work. Carries forward the one piece of process friction worth keeping: a layout decision taken from a measurement of that same layout has no fixed point. Five adaptive attempts failed that way before a media query replaced them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sandy Ghs (sandyghs)
force-pushed
the
fds/combobox-adoption
branch
from
August 27, 2026 10:55
699fd0d to
ed84ff3
Compare
…ement The defect was measured by another session on OpenCTI and is documented in OpenCTI PR #17884. Entry 45 records it on the OpenAEV side so this product carries a pointer rather than rediscovering it, and is explicit that the measurement, reproduction and analysis are not this branch's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three wrappers accepted `fullWidth` from 11 caller files and dropped it: the prop was never destructured and the string does not appear in any of the three. With MUI it reached the control; after conversion nothing applied it. Adding `w-full` to the trigger was not enough, and the measurement says why: the trigger's `width: 100%` resolves against the wrapper `<div>`, which carries no style of its own and is a flex item with no basis, so it shrink-wraps its child. The 100% is circular — the wrapper sizes to the trigger and the trigger fills the wrapper — and both collapse to the trigger's content. Measured on the scenario creation form, 760px track: the four selects were 146 / 190 / 81 / 173px wide. They are now 370px each, two per row, 370+370+20 = the full track — the arrangement the MUI version had. The width is applied only when a caller asks for it, so nothing changes for the sites that deliberately size a select to its content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Audited the 22 files whose pre-migration version carried `fullWidth` and whose trigger is bare today, element by element rather than by proximity: for each file the base's `<Select>` / `<TextField select>` openings were extracted and checked for the prop inside the same element. 18 files, 26 sites, declared `fullWidth` in the base and lost it in conversion — the library trigger sizes to its content. Those get `w-full` back. Four files kept as they are, because their base Selects carried no `fullWidth` and their content sizing is therefore faithful, not a regression: `ToolBar` (also out of scope), `SchedulingDialog`, `ReportingForm`, `ResponseDialog`. Verified on the trigger form: 760px track, trigger 760px. Where a site's own parent shrink-wraps, `w-full` alone will not be enough — same circular 100% as the form wrappers; none of the audited parents has that shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The library floors an option row at 48px on the mere PRESENCE of `renderOption` and its own default row is 32px. `AutocompleteField` passed one unconditionally, so all 17 of its consumers paid the floor. It now passes one only for `multiple`, which is the single case that needs a custom row — the checkbox. The tooltip and the ellipsis span go with it; no consumer sets `option.italic`, so that branch was dead. Audited the ten field wrappers that pass `renderOption` directly rather than through `AutocompleteField`: every one renders a real glyph or image (FlagOutlined, PlatformIcon, DomainOutlined, LabelOutlined, a 25px platform image, …). None is decorative, so none is removed — that is library gap #42, where a row with a glyph cannot avoid the floor. Measured, two screens and both layers: a single-select field is now `min-h-8`, 32px, 12px text, longest label fitting with no panel overflow; the arsenal Domains field, which is `multiple`, keeps `min-h-12` at 48px for its checkbox. Row height is identical in light and dark, as the floor is a static utility with no theme-conditional rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…not custom" This reverts commit 85d41af.
…de's 32px filigran-design-system #176 makes the row's floor declared rather than inferred from `renderOption`. Node 6899:15536 draws `Select Field Item` at 32px, `Multi Select Field Item` — the row carrying a checkbox — also at 32px, and only the two-line `Option Complex` at 48px. Twenty-seven sites are affected and none needs a change: seventeen through `AutocompleteField` and ten field wrappers that draw a glyph beside one label. They rendered at 48px because the wrapper passed a render prop, not because they asked for a taller row. Measured against the installed bytes, not deduced: `isOptionTwoLine` appears in the served Combobox module and the old ternary is gone; a plain field, a `multiple` field with its checkbox and an ornamented field all render `min-h-8`. No product site draws a two-line row, so nothing regresses from 48 to 32. LIBRARY-FEEDBACK #42 is rewritten: its earlier claim — that the product could not avoid the floor without dropping the glyph — was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The library publishes one module per source module since its #171, so the entry point is a barrel of re-exports and no longer carries Paper's border class. The assertion read the barrel and got `"use strict"`. What it guards is unchanged: the name the product writes must be a name the shipped Paper actually paints with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Combobox/Select surface is complete, so this takes the neighbouring
single-choice and toggle controls the library already ships.
Radio, two sites. Both keep a name the group can be addressed by, which the
library requires: its RadioGroup has no visible label of its own, so a consumer
must supply `aria-label` or `aria-labelledby`.
- Expectation group: the existing "Validation mode" FormLabel now carries an
id and names the group.
- Lessons template dialog: its `aria-labelledby` pointed at an id no element
had. It now names the dialog title. Each row's name and description move to
the library's own `label` and `description` props, replacing hand-built
Typography, and keep the separator the MUI list drew.
Checkbox and Switch, four sites: the two deprecated react-final-form wrappers
(one consumer each), the trigger form's event types, and the quick inject's
"All teams". Each control's label moves to the library prop that renders a real
`<label>`, so no accessible name is lost.
Two RadioGroups are deliberately left alone and listed for arbitration: neither
has a label that can be associated without inventing one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE until the design review signs off.
Adopts the library
ComboboxandSelectacross the product's selectionfields, and fixes the visual defects the review surfaced on the way. Pinned on
library
cd4b8ee(#155: controlledopen,startIcon,adornment).107 files. The wave grew well past its opening scope, so the sections below are
ordered by what a reviewer needs to judge rather than by when it landed.
Scope
Autocomplete→ComboboxAutocompleteinToolBarSelectfamily →SelectThe
Selectconversion was not in the original scope. It was added because apage mixing the two families reads as two different form languages, and the
review asked for the whole field surface at once. It lands over 11 commits,
grouped by what made each group awkward rather than by directory.
Deliberate UX changes — please read these two before the diff
The create button moved. On the threat arsenal list,
Créerused to sit inthe pagination row. The card view makes that row wider than the list view,
because it puts the sort select there: measured at a 1400px viewport, 47px of
the button was visible in list view and 0 in card view, with no horizontal
scroll to reach it. It now sits in the page header beside the export/import
icons — one instance, rendered in both views, 8px from the icon group. Covered
by
threatArsenal-list-header.spec.ts.The list toolbar is no longer one row at every width. It carried selection,
search, filters, sort, page size and pagination side by side; at 1400px the
pagination arrows sat at x=1471, off-window, with no horizontal scroll to reach
them. It now has one fixed break point at 1600px:
when no filter is applied.
The break point is a media query. Nothing measures anything at runtime and no
state depends on the width — five adaptive attempts were built and discarded
before this, each failing the same way: a measurement whose result depended on
the layout it produced. Above the break point the row wrapper drops to
display: contents, so its groups become items of the toolbar itself;orderputs the pagination last and
margin-left: autoholds it at the edge. Thegroups shrink rather than wrap, which is why the tight case fits: on the arsenal
list at 1600px the three groups use 993px of the 1106px available.
Both are product UX decisions taken by the design lead, not side effects of the
migration. Both are shared surfaces — the toolbar is used by 62 pages — so the
one-line container also wraps: a page carrying heavier slots falls back to two
rows instead of running past its container.
That safety net is load-bearing, not theoretical. Above the break point the card
view can still wrap, because it carries the sort select the list view does not:
measured in CI, at 1600px the card view wraps to two rows (44px offset) while
the list view sits on one. Nothing overflows and no horizontal scroll appears in
either case — the arrangement is the one-line one, the container simply needs a
second row for it. A single break point for both views was preferred over a
second mechanism.
Visual defects fixed, each traced to its own cause
InputLabel,which is a floating label (
position:absolute; transform:scale(.75)), so itrendered at a different size and offset from the library's own. All five now
use the library label; the six drawer rows share one 26px label-to-field
offset.
TagsFilterlaid its row out withfloat:leftand a5pxtop margin, which offset the chips 3px from their neighbours. The row isnow a centred flex row; all three chips land on centre 545.
MUI's
TablePaginationwraps its content in aToolbarcarrying its ownminHeight, and its captions are<p>elements carrying the browser's 1emvertical margins; separately the select-all
Checkboxcarries its ownpadding. Click targets after the change: checkbox 36×36, pagination arrows
32×32 — both above the 24×24 target-size minimum, so nothing was traded for
the height.
intent is separation of the panel from the page, not one absolute ordering.
The node is authored in dark, where the panel sits above its fields; in light
the hierarchy inverts, so the two modes take different tokens.
Notable in the Combobox wave
AutocompleteField'srenderOptionprop only ever tested its result fornull— a caller's node was discarded. It is nowhideOption, which is whatit always meant. Three callers follow; the wrapper's
variantprop goes withit, the library field having one style.
renderOption={option => option.label}are removed. The library floorsan option row at 48px whenever
renderOptionis passed, whatever it returns,so an identity one cost 16px per row for nothing.
open={false}with noonOpenChange: nopanel is mounted at all, and
Enter/ArrowDownkeep their native meaningin the input. Verified at the pointer.
FilterChipPopoverInputno longer drives selection from the option row. Thelibrary owns the row; the new selection is diffed against the old one. Two
visible consequences:
Enternow reaches the library's handler, and a rapiddouble click toggles twice instead of being absorbed.
placeholder, so the field reads as one thing feeding a token list.
Library gaps measured here, reported, not worked around
Recorded in
fds-migration/LIBRARY-FEEDBACK.md:Comboboxneeds a cause on every callback; one product siteexpresses that cause as an event's presence.
min-h-8, so a site pays only forwhat it renders. The 48px floor applies to 12 sites rendering one glyph plus
one line of text; the product cannot avoid it without dropping the glyph.
Selectrenders no wrapper of its own, so its label and triggerbecome siblings of whatever contains them. The most serious of the three.
ComboboxLabelhas norequired, so the two field families exposethe required marker differently, and therefore expose different accessible
names.
SelectandTab. Filed as a pointer only: the defect was measuredby another session on OpenCTI and belongs to OpenCTI PR #17884. Nothing here
compensates for it.
Known and deliberately not addressed in this PR
ToolBarAutocompletesites — settled as out of scope.SelectandTabinteraction — accepted for a later pass, tracked as gapLogin Credentials for Demo not working #45 pointing at OpenCTI PR #17884.
labelPositionaxis is not wired to any product site yet.order cannot serve two arrangements whose reading orders differ, and the
stacked arrangement — the common widths — is the one kept concordant.
Duplicating the pagination group per arrangement would fix it but needs 13
unscoped test locators re-scoped first, so it is deferred, not adopted.
needing to. One threshold for both views was preferred over a second
mechanism; the band renders the stacked arrangement, which is the reviewed
one.
fitting one line, for the same reason — one threshold, two views of different
widths. Measured, no overflow either way.
lead.
Verification
src/__tests__/utils/Cron.test.tsxfails 81 tests, reproduced on the basecommit and independent of timezone; that file is not touched here.
views, by the wrapper's computed
displayrather than by the number of visuallines; the chips row is asserted present only while a filter is applied.
both views, ascending and descending — 26 measurements, no overflow inside the
bar at any width, no group wrapping internally, and the break point landing
exactly on 1600.
break point;
leadingIconreturns 0 occurrences, confirming the rename tostartIcon.