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

fixed type problem of variable color #1952

Closed
wants to merge 2 commits into from
Closed

fixed type problem of variable color #1952

wants to merge 2 commits into from

Conversation

luoqiuyun
Copy link

I'm doing unit test of our web applications.
I got error messages as following: color.charAt is not a function

I debug the file: material-ui/lib/utils/color-manipulator.js
I found that the parameter "color" passed to function _decomposeColor(color) is not always of type String.

This bug may not bother the developer so much, but it bugs the unit testing.
I think this need to be fixed.

Thanks you.

QIu Yun

@oliviertassinari
Copy link
Member

I found that the parameter "color" passed to function _decomposeColor(color) is not always of type String.

I would rather fix this. Do you know where it's from?

@luoqiuyun
Copy link
Author

the bug locates in:

/node_modules/material-ui/lib/utils/color-manipulator.js
    _decomposeColor: function _decomposeColor(color) {
       color = String(color);           // add a line to force the "color" be a string, and fix the bug
        if (color.charAt(0) === '#')    // this is where the bug is

When the following methods calls it, they pass a parameter "color" that is not always a String

  _luminance: function _luminance(color) {
        color = this._decomposeColor(color);
  fade: function fade(color, amount) {
        color = this._decomposeColor(color);
  lighten: function lighten(color, amount) {
        color = this._decomposeColor(color);
  darken: function darken(color, amount) {
        color = this._decomposeColor(color);

@c0b41
Copy link

c0b41 commented Oct 22, 2015

This is not bug fix, problem raw theme variable empty end this fire error. Something raw theme variable default value fixed this problem.

@@ -79,6 +79,8 @@ module.exports = {

// Returns the type and values of a color of any given type.
_decomposeColor(color) {
color = String(color);
Copy link
Member

Choose a reason for hiding this comment

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

I think that you need the new

Copy link
Author

Choose a reason for hiding this comment

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

"new" added, and pushed. Thanks.

@luoqiuyun
Copy link
Author

I set the following:

          rawTheme:   {spacing: {desktopKeylineIncrement: 1},
                                palette: {textColor: "#000000",
                                 primary1Color: "#1111111",
                                 primary2Color: "#222222",
                                 alternateTextColor: "#333333",
                                 disabledColor: "#444444",
                                 borderColor: "#555555",
                                 accentColor: "#666666",
                                 accent1Color: "#777777",
                                 canvasColor: "#888888",
                                 default: "#999999"
                                }
                      },

error msg

  - TypeError: color.charAt is not a function
        at Object._decomposeColor (../node_modules/material-ui/lib/utils/color-manipulator.js:76:15)
        at Object.lighten (../node_modules/material-ui/lib/utils/color-manipulator.js:97:18)
        at render (../node_modules/material-ui/lib/flat-button.js:141:68)

@oliviertassinari
Copy link
Member

The error you describe means that something is wrong. IMO merging this PR is like hiding the problem. I would rather fix the issue. Hence, I'm closing this PR. I think that it's linked to a wrong usage of the theme.
Could you find that's wrong and open an issue? Would be awesome.

at render (../node_modules/material-ui/lib/flat-button.js:141:68) looks like a good place to start.

mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Nov 10, 2020
@zannager zannager added the customization: theme Centered around the theming features label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customization: theme Centered around the theming features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants