Skip to content

feat(data-weaver): Icons normalization - #464

Open
pablonoel wants to merge 5 commits into
datacommonsorg:mainfrom
pablonoel:ui-icons
Open

feat(data-weaver): Icons normalization#464
pablonoel wants to merge 5 commits into
datacommonsorg:mainfrom
pablonoel:ui-icons

Conversation

@pablonoel

@pablonoel pablonoel commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Overview

Standardizes icon assets by migrating various SVG icons to Material Symbols, using https://fonts.google.com/icons, as reference.

Changes Made

  • Update viewBox to 960x960 and path data
  • Replacing outdated icons like IconHand and IconLineGraphDouble
  • Iintroducing CSS variables (--icon-size-small and --icon-size-medium) to manage icon sizing consistently
  • Redrawing of custom icons: Narrative.tsx and Shapes.tsx to visually match the specs of the rest
  • Replacing import icons from download, to upload, and upload file
Screenshot 2026-09-08 at 12 56 03 PM Screenshot 2026-09-08 at 12 56 09 PM Screenshot 2026-09-08 at 3 03 49 PM

Testing Done

Describe the steps you took to test these changes (please also list commands ran if possible).

  • Unit tests passed
  • Integration tests passed
  • Manual verification

Checklist

  • I have followed the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.

Note: Only Maintainers can approve and merge PRs. Expected initial review time: 3 business days.

Add CSS vars --icon-size-small/--icon-size-medium (with tablet breakpoint) and replace hard-coded icon widths/heights across components. Standardize many primitive SVG icons to a new viewBox and updated path data. Remove legacy icons (IconHand, IconLineGraphDouble), add IconPanTool and IconLineAxis, and update usages (tools.tsx, menu_chart_options.tsx). Re-export IconExport/IconImport from download and update status indicator and various styles to use the new icon size vars.
Replace the re-export with an inline, props-forwarding IconExport component (adds aria-hidden and SVG attributes). Update icons with proper alignment and weight

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request standardizes icon assets by migrating various SVG icons to Material Symbols (updating their viewBox to 960x960 and path data), replacing outdated icons like IconHand and IconLineGraphDouble, and introducing CSS variables (--icon-size-small and --icon-size-medium) to manage icon sizing consistently. Feedback on these changes highlights two main issues: first, aliasing IconImport to IconDownload introduces a visual mismatch with IconExport, so a dedicated horizontal bracket-and-arrow SVG path is suggested; second, the .icon-status class in card.module.scss was incorrectly downsized to --icon-size-small and should instead use --icon-size-medium to preserve its original 24px proportions.

Comment thread dataweaver/apps/web/src/components/primitives/icons/import.tsx Outdated
Replace the previous re-export with a dedicated IconImport React component that renders the import SVG and forwards props. Update .icon-status in card.module.scss to use --icon-size-medium (was --icon-size-small) to enlarge the status icon.
Renamed dataweaver/apps/web/src/components/primitives/icons/import.tsx -> upload.tsx and updated the exported component to IconUpload with a new SVG/viewBox. Added a new IconUploadFile component (upload_file.tsx) for the file-upload variant. Updated import sites: the in-front-of-canvas import control now uses IconUpload, and the import menu uses IconUploadFile.
@pablonoel
pablonoel marked this pull request as ready for review September 8, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant