Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[system] Warns if the hex color contains trailing space #44538

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

siriwatknp
Copy link
Member

closes #44504

Test added.

@siriwatknp siriwatknp added package: system Specific to @mui/system dx Related to developers' experience labels Nov 25, 2024
@siriwatknp siriwatknp changed the title [system] Warn if the hex color contains trailing space [system] Warns if the hex color contains trailing space Nov 25, 2024
@mui-bot
Copy link

mui-bot commented Nov 25, 2024

Netlify deploy preview

https://deploy-preview-44538--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 5dba5e2

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

@siriwatknp what if we .trim() the input instead?

it('warns if the color contains space at the end', () => {
let result;
expect(() => {
result = hexToRgb('#aa0099 ');
Copy link
Member

@oliviertassinari oliviertassinari Nov 26, 2024

Choose a reason for hiding this comment

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

I think it would be great to test a high-level API, like alpha() instead. I don't see why hexToRgb() is public in the first place.

@siriwatknp
Copy link
Member Author

@siriwatknp what if we .trim() the input instead?

I don't think so. The point of this PR is to warn if the input is not correct. I am reluctant to process the input at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Related to developers' experience package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[system] decomposeColor() has null value when string has whitespace
4 participants