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

[ListItem] Add the ability to change the colour of the nested indicator icon #3654

Closed
adrianmcli opened this issue Mar 10, 2016 · 9 comments
Closed
Labels
component: list This is the name of the generic UI component, not the React module!

Comments

@adrianmcli
Copy link

This is a feature request/discussion concerning this component: http://www.material-ui.com/#/components/list

I came across this problem while trying to implement a dark theme. For the list component, I was able to easily set the background colour to be a dark colour as well as change the text to a light colour to maintain contrast.

However, a problem arises, when I have a nested list item. The nested indicator icon seems to only display in a dark colour, making it hard for the user to see it on a dark background.

See picture here:
dark indicator

Lines 606 and 607 of list-item.jsx shows us that this is because it's rendering the default icon component whenever the list is being collapsed or expanded.

      // Create a nested list indicator icon if we don't have an icon on the right
      if (needsNestedIndicator) {
        rightIconButtonElement = this.state.open ?
          <IconButton><OpenIcon /></IconButton> :
          <IconButton><CloseIcon /></IconButton>;
        rightIconButtonHandlers.onTouchTap = this._handleNestedListToggle;
      }

See source here.

I can override the icon by using the rightIcon prop, but it no longer flips up and down due to the fact that it's just a static icon.

My proposal is to add a nestedIndicatorColor prop so we can control the way the "generated" indicator looks on dark backgrounds.

Thoughts? If there is support, I will attempt a pull-request.

@sorahn
Copy link

sorahn commented Apr 4, 2016

A quick fix would be to override the theme color for 'listItem.rightIcon'

@tintin1343
Copy link
Contributor

@adrianmc : were you able to solve the problem using the above solution? Menu and ListItem component is currently too complexed and a rewrite is being planned.

If you were able to get the problem fixed by overriding the 'listItem.rightIcon' color, I would close this for now. Do let me know.

@adrianmcli
Copy link
Author

@tintin1343 Unfortunately, I didn't get around to it. I just gave up and omitted the indicator. It's not a big deal, just something to think about on the next rewrite, I guess. Thanks anyway.

@tintin1343
Copy link
Contributor

@adrianmc : Thanks for pointing it out. SInce the problem was not solved I am reopening this issue so that we can keep a track of changes we need to do for the redesign.

@tintin1343 tintin1343 reopened this Apr 25, 2016
@tintin1343 tintin1343 added bug 🐛 Something doesn't work Refactoring and removed bug 🐛 Something doesn't work labels Apr 25, 2016
@scrambled2k3
Copy link

scrambled2k3 commented Aug 19, 2016

@adrianmc @sorahn @tintin1343 : That solution did not work as it looks like the theme color is only being applied when a rightIcon is sent through the ListItem props. It is not applied to the icon the ListItem uses by default.

This is from the ListItem class:

if (rightIcon) {
      const additionalProps = {
        color: rightIcon.props.color || this.context.muiTheme.listItem.rightIconColor,
      };
      this.pushElement(
        contentChildren,
        rightIcon,
        Object.assign({}, styles.icons, styles.rightIcon),
        additionalProps
      );
    }

The "rightIcon" it is referring to is what it pulls in from props.

@adrianmcli
Copy link
Author

To be honest, I just gave up and went with React Toolbox. They have a
smaller library, but they're a lot more themeable.

On Aug 19, 2016 9:24 PM, "Dusty Sirmans" [email protected] wrote:

@adrianmc https://github.com/adrianmc @sorahn
https://github.com/sorahn : That solution did not work as it looks like
the theme color is only being applied when a rightIcon is sent through the
ListItem props. It is not applied to the icon the ListItem uses by default.

This is from the ListItem class:
if (rightIcon) {
const additionalProps = {
color: rightIcon.props.color || this.context.muiTheme.
listItem.rightIconColor,
};
this.pushElement(
contentChildren,
rightIcon,
Object.assign({}, styles.icons, styles.rightIcon),
additionalProps
);
}


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3654 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5lw8pDsqa0r8v7B9TenrN0CJEjT3qmks5qha5wgaJpZM4Htxsl
.

@oliviertassinari oliviertassinari added the component: list This is the name of the generic UI component, not the React module! label Dec 18, 2016
@amirmohsen
Copy link

amirmohsen commented Feb 17, 2017

You can always use css to target the svg tag's color. That's what I did.

@cristianocca
Copy link

So now that this is closed, how to properly change this without affecting the styles globally for every list and without playing around with CSS?

@oliviertassinari
Copy link
Member

oliviertassinari commented May 23, 2017

This was closed as we now expose a classes property on the next branch so users can have access and apply CSS to all the elements of Material-UI. Effectively meaning that everything can be customized. Things haven't changed on the master branch. Issues will be slowly closed as problems are solved on the next branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: list This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

7 participants