Skip to content

refactor: consolidate SearchSvg icon component into centralized pattern - #2358

Open
MamtaKumari2006 wants to merge 5 commits into
appbaseio:nextfrom
MamtaKumari2006:next
Open

MamtaKumari2006 wants to merge 5 commits into
appbaseio:nextfrom
MamtaKumari2006:next

Conversation

@MamtaKumari2006

@MamtaKumari2006 MamtaKumari2006 commented Sep 25, 2026 •

Copy link
Copy Markdown

Proposed Changes

Closes #2324

Checklist

  • Describe the proposed changes and how it'll improve the library experience.
  • Please make sure that there are no linting errors in the code.
  • Add a demo video/gif/screenshot to explain how did you test the fix.
  • If it is a global change, try to add any side effects that it could have.
  • Create a PR to add/update the docs (if needed).
  • Create a PR to add/update the storybook (if needed).

Copilot AI lite review requested due to automatic review settings September 25, 2026 08:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved build, lint, snapshot, dependency, and consolidation issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity · 1 Low severity

Open (4)
What changed in this PR

This PR begins centralizing SVG icon rendering and updates site build/editor configuration.

Changes:

  • Adds a shared search icon registry and renderer.
  • Updates the legacy SearchSvg wrapper.
  • Adjusts site scripts and VS Code JSON formatting.
File Summary
site/​package.json Updates site build and start scripts.
packages/​web/​src/​components/​shared/​SearchSvg.js Delegates search icon rendering.
packages/​web/​src/​components/​shared/​Icons.js Adds centralized icon rendering infrastructure.
.vscode/​settings.json Configures JSON formatting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

SearchSvg.propTypes = {
style: types.style,
};
import { SearchSvg } from './icons';
Comment on lines +17 to +21
export const ICONS_PATHS = {
search: `M6.02945,10.20327a4.17382,4.17382,0,1,1,4.17382-4.17382A4.15609,4.15609,
0,0,1,6.02945,10.20327Zm9.69195,4.2199L10.8989,9.59979A5.88021,5.88021,
0,0,0,12.058,6.02856,6.00467,6.00467,0,1,0,9.59979,10.8989l4.82338,
4.82338a.89729.89729,0,0,0,1.29912,0,.89749.89749,0,0,0-.00087-1.29909Z`,
Comment on lines +31 to +37
<svg
alt={name}
className={`${name}-icon`}
height="12"
width="12"
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 15 15"
Comment thread packages/web/src/components/shared/Icons.js Outdated
@MamtaKumari2006 MamtaKumari2006 changed the title refactor: consolidate duplicate SVG icon components into centralized pattern refactor: consolidate SearchSvg icon component into centralized pattern Sep 27, 2026
@MamtaKumari2006

Copy link
Copy Markdown
Author

Hi @siddharthlatest, I have resolved the casing issue, formatting violations, and successfully updated all snapshot tests locally (yarn test -u). Everything is passing now. Please review and approve the workflow run. Thanks!

@siddharthlatest

siddharthlatest commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

@MamtaKumari2006 Thanks for taking this on. The search re-export is the right shape, but #2324 is the whole set of near-duplicate SVG icons, and this branch only moves the search icon. A few unrelated files should come out, and the helper needs to cover the rest of the icons.


Thanks for the snapshot update. ReactiveSearch Snapshot Tests / build (22.x) is green (about 3 minutes), and the branch merges cleanly with base. The earlier failure on b685cbc1 (39 snapshots in SearchBox and AIAnswer, search path d whitespace) is resolved on this head.

The remaining red check is Labeler / label. It fails in about 6 seconds on fork PRs because that workflow cannot write labels (Resource not accessible by integration). That is a permissions limit, not a product failure, and it does not need a code change.

1. Drop everything that is not the icon change. Remove .vscode/settings.json, the site/package.json NODE_OPTIONS / cross-env script change (cross-env is not added as a dependency), and the regenerated site/dist/common.bundle.js and site/dist/main.js. Also remove the Hindi/English source comment in Icons.js.

2. Replace the search-only helper. SvgIcons hardcodes height="12", viewBox="0 0 15 15", and the search scale, so it cannot absorb the other icons. Use one registry plus a shared icon component, and keep thin default re-exports so existing imports keep working.

3. Move the duplicate modules in packages/web/src/components/shared/. That is AutofillSvg, CancelSvg, DownloadSvg, ListenSvg, MicSvg, MuteSvg, SearchSvg, ThumbsDownSvg, and ThumbsUpSvg. Leave CustomSvg.js alone. It is not a path clone: it dispatches a custom icon, a recent-search icon, or a promoted-search icon.

4. Keep the current DOM shape for the voice icons. Mic, mute, and listen return a fragment with <Global> (and <defs> for listen) beside the <svg>, not inside it. A single <svg> wrapper that puts <Global> in children changes the element tree. Keep <Global> outside <svg>.

5. Re-run snapshot tests after that wider move. The green run covers the current search-icon diff. Any further d or DOM edit needs matching snapshot updates and another green ReactiveSearch Snapshot Tests run. Labeler can stay red.

6. Make the description match the diff. Say that this PR closes #2324 by consolidating those nine modules, and list the files. The current body still narrows the issue to the search icon.

@MamtaKumari2006

Copy link
Copy Markdown
Author

Hi @siddharthlatest, thank you for the detailed review! I'll clean up the unrelated files, consolidate all 9 icon modules into the shared registry, preserve the voice icon DOM structure, and re-run snapshot tests. Will push the updated changes soon.

@MamtaKumari2006

Copy link
Copy Markdown
Author

Hi @siddharthlatest,

I have addressed all the feedback points:

  1. Consolidated all 9 duplicate SVG modules into Icons.js with thin default re-exports.
  2. Preserved the <Global> DOM structure for the voice icons.
  3. Left CustomSvg.js untouched.
  4. Cleaned up site/package.json and comments.
  5. Updated all snapshots locally (yarn test -u) — all 15 test suites are passing.

The PR is ready for your review. Thanks!

This branch has not been deployed

No deployments
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.

refactor: consolidate 12 near-identical SVG icon components into one pattern

3 participants