Conversation
dosubot
bot
added
the
size:L
Summary of ChangesHello @hyoban, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors how icons are managed and used within the project by introducing a new ESLint rule and integrating Tailwind CSS for icon styling. The primary goal is to standardize icon usage, promote a utility-first approach, and potentially improve frontend performance and maintainability by leveraging Tailwind's capabilities for icons. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new system for using CSS icons with Tailwind, which is a great improvement for consistency and performance. A new ESLint rule prefer-tailwind-icon is added to encourage the adoption of this new system by suggesting replacements for old icon components. The implementation is solid, but I've found a couple of areas in the new ESLint rule that could be improved for robustness.
There was a problem hiding this comment.
Pull request overview
This PR introduces CSS-based icons using Tailwind CSS icon utilities to replace component-based icon libraries. It adds a new ESLint rule to encourage the migration from icon library components to Tailwind CSS classes.
Changes:
- Adds Tailwind CSS icons plugin with support for Heroicons, Remix Icons, and custom SVG collections
- Implements a custom ESLint rule (
prefer-tailwind-icon) to suggest replacing icon components with Tailwind classes - Adds necessary dependencies for icon handling (
@egoist/tailwindcss-icons,iconify-import-svg, and icon collection packages)
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/tailwind-common-config.ts | Configures Tailwind icons plugin with icon collections and custom SVG support |
| web/package.json | Adds dependencies for Tailwind icons and icon collections |
| web/eslint.config.mjs | Enables the new prefer-tailwind-icon rule for TSX files |
| web/eslint-rules/rules/prefer-tailwind-icon.js | Implements ESLint rule to detect and suggest replacing icon components with Tailwind classes |
| web/eslint-rules/index.js | Registers the new prefer-tailwind-icon rule in the plugin |
Files not reviewed (1)
- web/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- web/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- web/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "esbuild": "0.27.2", | ||
| "eslint": "9.39.2", | ||
| "eslint-plugin-better-tailwindcss": "4.1.1", | ||
| "eslint-plugin-better-tailwindcss": "https://pkg.pr.new/hyoban/eslint-plugin-better-tailwindcss@c0161c7", |
There was a problem hiding this comment.
The eslint-plugin-better-tailwindcss dependency has been changed from a version number (4.1.1) to a direct GitHub tarball URL pointing to a specific commit (c0161c7) on pkg.pr.new. This is concerning because:
- It uses an unstable package source (pkg.pr.new is for preview builds from PRs)
- The specific commit may not be guaranteed to remain available long-term
- This makes the build less reproducible and potentially less stable
- The PR description doesn't explain why this change is necessary
This change should either be reverted to use a stable version from npm, or a clear explanation should be provided in the PR description about why the preview build is necessary and what the plan is to migrate back to a stable release.
| "eslint-plugin-better-tailwindcss": "https://pkg.pr.new/hyoban/eslint-plugin-better-tailwindcss@c0161c7", | |
| "eslint-plugin-better-tailwindcss": "4.1.1", |
| }), | ||
| ...importSvgCollections({ | ||
| source: path.resolve(_dirname, 'app/components/base/icons/assets/vender'), | ||
| prefix: 'custom-vender', |
There was a problem hiding this comment.
The directory name 'vender' appears to be a spelling error - the correct spelling is 'vendor'. While this spelling is used consistently throughout the codebase (as evidenced by the existing directory structure at 'app/components/base/icons/assets/vender'), it's still technically incorrect. Since this is a new configuration being added, it would be a good opportunity to fix the spelling, though doing so would require renaming the directory and updating all references throughout the codebase.
| prefix: 'custom-vender', | |
| prefix: 'custom-vendor', |
| "lint": "eslint --cache --concurrency=auto", | ||
| "lint:ci": "eslint --cache --concurrency 2", |
There was a problem hiding this comment.
The concurrency value has been changed from the double-quoted string "auto" to an unquoted value auto, and from 3 to 2 for the CI environment. While the change from quoted to unquoted is fine (both work), the reduction in concurrency for CI from 3 to 2 is not explained in the PR description. This change appears unrelated to the main purpose of introducing CSS icons and should either be explained or reverted.
1 participant
Important
Fixes #<issue number>.Summary
Fixes #30797
rework #31551
Screenshots
Checklist
make lintandmake type-check(backend) andcd web && npx lint-staged(frontend) to appease the lint gods