Skip to content

Standardize Icon Dimensions Using DEFAULT_WIDTH and DEFAULT_HEIGHT#1592

Closed
Rajesh-Nagarajan-11 wants to merge 4 commits into
layer5io:masterfrom
Rajesh-Nagarajan-11:icon-default-dimensions
Closed

Standardize Icon Dimensions Using DEFAULT_WIDTH and DEFAULT_HEIGHT#1592
Rajesh-Nagarajan-11 wants to merge 4 commits into
layer5io:masterfrom
Rajesh-Nagarajan-11:icon-default-dimensions

Conversation

@Rajesh-Nagarajan-11
Copy link
Copy Markdown
Member

Notes for Reviewers

Ensured every icon consistently defaults to DEFAULT_WIDTH and DEFAULT_HEIGHT.

Many icons were either missing fallback values entirely or using hardcoded dimensions (such as '24px' or '32px'). This update standardizes them all for consistency and follows the same implementation pattern used across other icons.

Signed commits

  • Yes, I signed my commits.

Rajesh-Nagarajan-11 and others added 3 commits May 28, 2026 19:40
Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Copy link
Copy Markdown
Contributor

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

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 the default dimensions of various icon components by importing and applying DEFAULT_WIDTH and DEFAULT_HEIGHT constants to their width and height props. Additionally, some prop types were updated to be optional and accept both numbers and strings. The review feedback suggests formatting improvements across several icon files to ensure that each destructured prop is placed on its own line when split across multiple lines, enhancing code readability and consistency.

Comment thread src/icons/FilledCircleIcon.tsx Outdated
Comment thread src/icons/InfoOutlined/InfoOutlined.tsx Outdated
Comment thread src/icons/Kanvas/KanvasIcon.tsx Outdated
Comment thread src/icons/Person/PersonIcon.tsx Outdated
Comment thread src/icons/Share/ShareLineIcon.tsx Outdated
Comment thread src/icons/Teams/TeamsIcon.tsx Outdated
Signed-off-by: Rajesh-Nagarajan-11 <rajeshnagarajan36@gmail.com>
Copy link
Copy Markdown
Member

@Sbragul26 Sbragul26 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Junnygram
Copy link
Copy Markdown
Contributor

@Rajesh-Nagarajan-11 Replacing unique default dimensions with global defaults (DEFAULT_WIDTH / DEFAULT_HEIGHT) will cause layout shifts in consuming apps:

  1. Class Icons (CommunityClassIcon, OfficialClassIcon, VerificationClassIcon): These were custom-sized at 16x13 for compact inline display. Standardizing them to the global default (usually 24x24) will make them much larger and potentially break inline alignments.
  2. Learning / Challenges Icons: Downsized from their original 32px default.
  3. GridView / TableView Icons: Previously had a tall custom aspect ratio (24x28.8), now forced to standard square defaults.

Should we keep custom default constants for these specific sub-families of icons, or are we expecting consuming products to explicitly pass overrides at every usage site?

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.

3 participants