LoicMahieu/material-ui-color-picker

The PickerDialog disappears when scrolling if the ColorPicker is contained in an element with a scroll bar

magjac opened this issue · 4 comments

I have two cases:

  1. A simplifed "real" case where the ColorPickers are contained in a Drawer with scroll bar (auto). If the PickerDialog is just partly visible when it opens, trying to scroll it into view makes it disappear which makes it impossible to use.

  2. The ColorPickers are contained in a div with scroll bar (auto). This shows the same problem when clicking the scroll bar although in this case the PickerDialog is fully visible before clicking the scroll bar because it "sticks out" below the div (which BTW is not what I want), so there's not the same need to use the scroll bar.

I think the culprit is this line:

style={{ position: 'fixed', top: '0px', right: '0px', bottom: '0px', left: '0px' }}

which seems to place an overlay over the whole document (including the scroll bar) to which a click event is attached that closes the PickerDialog.

I think a solution would be to use "width" instead of "right" and make it as wide as the ColorPicker instead. That works in the debugger at least.

I would be happy to supply such a PR, if that's the intended behavior, but I don't know how to run and test it locally. I'm fairly new to HTML/CSS/Javascript and web development in general and started learning React and Material-UI less than a month ago so I'm still struggling to understand what the code I've copy-and-pasted into my application from different examples actually does. Apologies if I'm using the ColorPicker in the wrong way. Please correct me if I've done something wrong.

Case 1:

import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { withStyles } from '@material-ui/core/styles';
import Drawer from '@material-ui/core/Drawer';
import ColorPicker from 'material-ui-color-picker'

const styles = {
  mystyle: {
    width: '250px', // 225px is the picker dialog width
    height: "300px", // less than half the screen height
    overflowY: 'auto',
  },
};

const pickers = [
  'pickerA',
  'pickerB',
  'pickerC',
  'pickerD',
  'pickerE',
  'pickerF',
  'pickerG',
];

class App extends Component {
  render() {
    const { classes } = this.props;
    return (
      <Drawer
          variant="persistent"
          anchor='left'
          open='true'
          classes={{
            paper: classes.mystyle,
          }}
        >
        {pickers.map((picker) =>
            <ColorPicker
              name={picker}
              label={picker}
              key={picker}
              defaultValue='#123456'
              onChange={() => console.log('change')}
            />
          )}
      </Drawer>
    );
  }
}


App.propTypes = {
  classes: PropTypes.object.isRequired,
};

export default withStyles(styles)(App);

Case 2:

import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { withStyles } from '@material-ui/core/styles';
import ColorPicker from 'material-ui-color-picker'

const styles = {
  mystyle: {
    width: '250px', // 225px is the picker dialog width
    height: "300px", // less than half the screen height
    overflowY: 'auto',
  },
};

const pickers = [
  'pickerA',
  'pickerB',
  'pickerC',
  'pickerD',
  'pickerE',
  'pickerF',
  'pickerG',
];

class App extends Component {
  render() {
    const { classes } = this.props;
    return (
      <div id="mydiv" className={classes.mystyle}>
        {pickers.map((picker) =>
            <ColorPicker
              name={picker}
              label={picker}
              key={picker}
              defaultValue='#123456'
              onChange={() => console.log('change')}
            />
          )}
      </div>      
    );
  }
}


App.propTypes = {
  classes: PropTypes.object.isRequired,
};

export default withStyles(styles)(App);

Hi @magjac

Integrate this module in a scrolling container is not the use I had planned ;) But indeed it could be useful at time.
Unfortunately I have no time at the moment to help you but I will do my best.

The fixed div in the PickerDialog is a quick and dirty way to handle click outside of the dialog. It must be a better way to do that.

I invite you to send a PR, even if it is dirty and not completely working. It will be a start for discussion.

Thanks for your consideration. I ended up replacing the material-ui-color-picker component with a custom implementation based on the SketchExample from https://casesandberg.github.io/react-color/#examples, because I wanted to add a swatch and some other stuff and to get better control over the styling. I'm not experienced enough to create a general customizable component. Sorry about that.

I'll post a link to my implementation here when I've pushed it to github in the event you want to have a look at it later.

Again thanks. 🙂

Here is my implementation.

The state has been lifted up to the parent component here and here

Thanks for sharing. Not enough time to work on this.
graphviz-visual-editor looks amazing. Good work!