[Data Importer] Highlight the expanded mapping and align its destination column - #685
Open
idaiv wants to merge 2 commits into
Open
[Data Importer] Highlight the expanded mapping and align its destination column#685idaiv wants to merge 2 commits into
idaiv wants to merge 2 commits into
Conversation
…ion column Highlights the open mapping section. Collapsed and expanded sections looked identical apart from the chevron, so nothing marked the one being edited. The expanded panel now carries the primary border from the design. The rule needs !important. The SDK draws the panel border from .ant-collapse.collapse-item--theme-default.collapse-item--bordered, which outranks the wrapper selector, and it is a border shorthand rather than a border-color. The drag-and-drop rules alongside it are !important for the same reason and are one class more specific, so dragging still takes over the border while it is in progress. Swaps the settings gear for the transformation icon on the Advanced button and on the badge that sits above the arrow when an advanced setup exists, matching the design. The badge was already wired to isAdvanced and only had the wrong icon. The icon already ships in the studio-ui-bundle library, so nothing has to be added there. arrowGearIcon is renamed to arrowAdvancedIcon to match what it now renders. Aligns the advanced destination text with the rest of the row. It used fontSizeSM, which resolves to 10px against the 12px every control around it uses, and it filled only its line box at the top of the slot, so it sat above the vertical center of the source Select and the arrow. It now uses fontSize and occupies the same controlHeight box as the Select it replaces, centered within it. The "Requires advanced setup" hint shares that slot and had both problems, so it follows the same rules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Contributor
There was a problem hiding this comment.
Pull request overview
Verdict: Needs changes.
Updates mapping panels with expanded-state highlighting, transformation icons, and improved destination alignment.
Changes:
- Highlights expanded mapping panels.
- Replaces settings icons with transformation icons.
- Aligns destination and warning text with standard controls.
Review assessment:
- Root cause is addressed for fixed-height controls, but wrapped multi-source Selects still misalign destination text (
mapping-step.styles.tsx:444,456). - Relevant call sites and style renames are covered.
- Changes are correctly scoped with no API or backward-compatibility impact.
- No visual-regression infrastructure was identified; remaining risk is wrapped multi-source layouts.
Reviewed changes
Copilot reviewed 13 out of 32 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
mapping-step.styles.tsx |
Adds highlighting, renamed icon styling, and alignment rules. |
mapping-item.tsx |
Applies expanded-panel styling conditionally. |
mapping-item-content.tsx |
Uses the transformation icon. |
arrow-column.tsx |
Updates the advanced-state icon and style name. |
Suppressed comments (1)
assets/studio/js/src/modules/data-importer/components/tabs/steps/mapping-step/mapping-step.styles.tsx:456
- The warning state is specifically reachable with multiple sources, whose Select can wrap and make this row taller than
controlHeight. Because this hint only has a minimum height, it stays top-aligned while the arrow centers in the stretched row. Let it fill the available destination slot as well.
min-height: ${token.controlHeight}px;
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| justify-content: center; | ||
| /* Occupies the same box as the Select it replaces, so the text lands on the same | ||
| vertical center as the source Select and the arrow between them. */ | ||
| min-height: ${token.controlHeight}px; |
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
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.



Changes in this pull request
Resolves #645
Three related fixes in the Data Setup → Mapping step.
1. Highlight the expanded mapping section
Collapsed and expanded sections looked identical apart from the chevron, so nothing marked the one being edited. The expanded panel now carries the primary border from the design (
colorPrimaryBorder, the token named in Figma, which resolves to#d3adf7).expandedwas already a controlled prop drivingPanel active, so this adds no state — just a class derived from it, on the wrapper that already owns the panel’s border colour.Why
!important: the SDK draws the panel border from.ant-collapse.collapse-item--theme-default.collapse-item--bordered— four classes, and abordershorthand. The wrapper selector is two classes, so a plainborder-colorthere never applies. The drag-and-drop rules directly above are!importantfor exactly this reason. They are also one class more specific, so a drag still takes over the border while it is in progress, which is the behaviour we want.2. Transformation icon for the advanced setup
Swaps the settings gear for the
transformationicon on the Advanced button and on the badge above the arrow shown when an advanced setup exists.The badge was already wired to
isAdvancedand only had the wrong icon — no new logic. The icon already ships in the studio-ui-bundle library and is registered inicon-library/index.ts, so nothing needs to be added there and this PR has no cross-repo dependency.arrowGearIconis renamed toarrowAdvancedIconto match what it now renders.3. Align the advanced destination column
Two problems in the same slot:
fontSizeSM→ 10pxfontSize→ 12px, matching the Select beside itcontrolHeight(32px) box as the Select it replaces, centredThe column itself was already the right height — 56px, the same as the source column (label + 4px gap + 32px control). What was off was the content inside it, which sat above the vertical centre of the source Select and the arrow. Using
token.controlHeightrather than a literal keeps it tied to the same token the Select derives its height from, and matches how the arrow column already aligns itself ("with the vertical center of the first Select").The "Requires advanced setup" hint occupies that identical slot and had both problems, so it follows the same rules — otherwise it would have looked misaligned in the warning state.
Additional info
Scope: 4 files, styling and icon names only. No behaviour, API or data changes.
Verification:
eslint✅ ·tsc --noEmit✅Exercised against the
car-importconfig in the demo, checking that the compiled bundle contains the new rules rather than trusting the build log.Not covered by tests: these are visual changes with no assertable behaviour; the repo has no snapshot or visual-regression setup for this step.