mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.53k stars 32.19k forks source link

weird alignment with table header and rows #3467

Closed luisrudge closed 7 years ago

luisrudge commented 8 years ago

Because the scrollbar appears in the right side of the table, the header and rows are wrongly aligned

image

alitaheri commented 8 years ago

Yeah I saw this before, it's because the header and rows use different table elements

mbrookes commented 8 years ago

I think they're in a separate table so that the components can be used independently which going to make this hard to fix.

What browser is this? Any modern browser that I've tried (Chrome, Safari, Firefox) overlays the scrollbar without adjusting the table width.

luisrudge commented 8 years ago

Yeah, I saw the two different tables as well, so I figured it out that was the problem. I'm not sure how to fix though :(

I'm on Chrome

mbrookes commented 8 years ago

What OS? I'd forgotten that it is OS X not the browser that provides the overlaid scrollbar.

tintin1343 commented 8 years ago

@luisrudge : Is this resolved for you or still a problem?

luisrudge commented 8 years ago

Let me check if the latest beta fixes the issue

luisrudge commented 8 years ago

It's not fixed, but it only happens when you use fixed header and a fixed height for the table: image

<Table fixedHeader height="10vh">

tintin1343 commented 8 years ago

@luisrudge : This looks fine to me. The header here is fixed right? Some of them are slightly off. Can you not apply inlineStyles and fix them?

luisrudge commented 8 years ago

It's not right, this is the expected result (it works when it's not fixed): image

tintin1343 commented 8 years ago

@luisrudge : Alright, Thanks! Will look into it.

luisrudge commented 8 years ago

do you have something setup to create repros? Like a codepen or something? That would be insanely cool.

tintin1343 commented 8 years ago

@luisrudge : AFAIK right now we don't. But I think we are planning on doing something like that.

CumpsD commented 8 years ago

Any ideas on how to fix this? Windows Chrome doesnt overlay the scrollbar with the fixedHeader causing the above effect.

I've tried playing with calculating scrollbar size and then offsetting the headers with that amount, but it's fragile at best :(

luisrudge commented 8 years ago

I gave up the fixed header :(

CumpsD commented 8 years ago

I am currently using scrollbar-width (from NPM)

In my start app.js I have:

import scrollbarWidth from 'scrollbar-width'

window.scrollbarWidth = scrollbarWidth()

And in the views with a grid I have a rather elaborate setup, take a look at how resize, styles, isScrollbarVisible play together:

import React from 'react'
import { withRouter } from 'react-router'
import { throttle } from 'lodash'
import Immutable from 'immutable'
import moment from 'moment'

import { Table, TableBody, TableHeader, TableHeaderColumn, TableRow, TableRowColumn } from 'material-ui/Table'
import IconButton from 'material-ui/IconButton'
import IconMenu from 'material-ui/IconMenu'
import MenuItem from 'material-ui/MenuItem'

import ShoppingIcon from 'material-ui/svg-icons/action/shopping-cart'
import SettingsIcon from 'material-ui/svg-icons/action/settings'
import EditIcon from 'material-ui/svg-icons/editor/mode-edit'
import DeleteIcon from 'material-ui/svg-icons/action/delete'
import MenuIcon from 'material-ui/svg-icons/navigation/more-vert'

import { FormattedMessage } from 'react-intl'

import * as Spacing from 'material-ui/styles/spacing'
import * as Colors from 'material-ui/styles/colors'

class Domains extends React.Component {
  constructor (props, context) {
    super(props, context)
    this.log = this.context.app.logger.init('admin-ui:domains:component')

    this.log('Initializing Domains.', props)

    if (this.props.activeAccount !== 0) {
      this.props.fetchDomains(this.props.activeAccount)
    }

    this._buildMenu = this._buildMenu.bind(this)
    this._buildDomainItems = this._buildDomainItems.bind(this)
    this._calculateExpiryDate = this._calculateExpiryDate.bind(this)
    this._onCellClick = this._onCellClick.bind(this)
    this._onResize = this._onResize.bind(this)
    this._onResizeThrottled = throttle(this._onResize, 10)
  }

  componentWillMount () {
    this.setState({
      windowHeight: window.innerHeight,
      scrollBarVisible: false
    })
  }

  componentDidMount () {
    this._onResize()
    window.addEventListener('resize', this._onResizeThrottled)
  }

  componentWillReceiveProps (nextProps) {
    if (nextProps.activeAccount !== this.props.activeAccount) {
      this.props.fetchDomains(nextProps.activeAccount)
    }
  }

  shouldComponentUpdate (nextProps, nextState) {
    return (
      nextProps.domains !== this.props.domains ||
      nextProps.currentLocale !== this.props.currentLocale ||
      nextState.windowHeight !== this.state.windowHeight ||
      nextState.scrollBarVisible !== this.state.scrollBarVisible)
  }

  componentDidUpdate (prevProps, prevState) {
    let scrollBarVisible = this.refs.table.isScrollbarVisible()

    if (prevState.scrollBarVisible !== scrollBarVisible) {
      this.log('Scrollbar visibility changed.', scrollBarVisible)
      this._onResize()
      this.render()
    }
  }

  componentWillUnmount () {
    window.removeEventListener('resize', this._onResizeThrottled)
  }

  _getStyles (scrollBarVisible) {
    return {
      root: {
        paddingTop: Spacing.desktopKeylineIncrement
      },
      header: {
        fontWeight: 'normal',
        fontSize: 13
      },
      headerRight: {
        fontWeight: 'normal',
        fontSize: 13,
        paddingLeft: scrollBarVisible ? 24 - (window.scrollbarWidth / 2) : 24
      },
      row: {
        borderBottom: '0px'
      }
    }
  }

  render () {
    // this.log('Rendering Domains.', this.props)

    const styles = this._getStyles(this.state.scrollBarVisible)

    let domains = this._buildDomainItems(this.props.domains, styles)

    // 64: top navbar, 58: table header, 2: borders
    return (
      <div style={styles.root}>
        <Table fixedHeader multiSelectable={false} selectable={false} onCellClick={this._onCellClick} height={(this.state.windowHeight - 64 - 58 - 2).toString()} ref={"table"}>
          <TableHeader enableSelectAll={false} displaySelectAll={false} adjustForCheckbox={false}>
            <TableRow>
              <TableHeaderColumn style={styles.header}><FormattedMessage id={"domains.name"} /></TableHeaderColumn>
              <TableHeaderColumn style={styles.headerRight}><FormattedMessage id={"domains.expiryDate"} /></TableHeaderColumn>
            </TableRow>
          </TableHeader>

          <TableBody deselectOnClickaway={false} showRowHover stripedRows={false} displayRowCheckbox={false}>
            {domains}
          </TableBody>
        </Table>
      </div>)
  }

  _buildDomainItems (domains, styles) {
    if (domains && domains.size > 0) {
      return domains.map(x => (
        <TableRow key={x.id} style={styles.row}>
          <TableRowColumn className={"mui-table-row-column-domain-name"}>{x.name}</TableRowColumn>
          <TableRowColumn className={"mui-table-row-column-domain-name"}>{this._calculateExpiryDate(x.expires_on)}</TableRowColumn>
        </TableRow>))
    } else {
      return (<div />)
    }
  }

  _calculateExpiryDate (dateText) {
    // 'YYYY-MM-DDTHH:mm:ss.SSSZ'
    var date = moment(dateText, moment.ISO_8601)

    // return date.fromNow()
    return date.format('LL')
  }

  _buildMenu () {
    const menuIcon = (
      <IconButton>
        <MenuIcon color={Colors.grey400} />
      </IconButton>
    )

    return (
      <IconMenu iconButtonElement={menuIcon}>
        <MenuItem value={"1"} leftIcon={<ShoppingIcon />} primaryText={this.context.app.i18n('domains.renewDomain')} />
        <MenuItem value={"2"} leftIcon={<EditIcon />} primaryText={this.context.app.i18n('domains.editDomainRecords')} />
        <MenuItem value={"3"} leftIcon={<SettingsIcon />} primaryText={this.context.app.i18n('domains.changeDomainSettings')} />
        <MenuItem value={"4"} leftIcon={<DeleteIcon />} primaryText={this.context.app.i18n('domains.deleteDomain')} />
      </IconMenu>
    )
  }

  _onResize () {
    this.setState({
      windowHeight: window.innerHeight,
      scrollBarVisible: this.refs.table.isScrollbarVisible()
    })
  }

  _onCellClick (rowNumber, columnNumber) {
    this.props.router.push('/domain/' + this.props.domains.get(rowNumber).id)
  }
}

Domains.displayName = 'Domains'

Domains.propTypes = {
  currentLocale: React.PropTypes.string.isRequired,
  activeAccount: React.PropTypes.number,
  domains: React.PropTypes.objectOf(Immutable.List),

  router: React.PropTypes.shape({
    push: React.PropTypes.func.isRequired
  }).isRequired,

  fetchDomains: React.PropTypes.func.isRequired
}

Domains.contextTypes = {
  app: React.PropTypes.object
}

export default withRouter(Domains)
oliviertassinari commented 7 years ago

We have been porting the component on the v1-beta branch. We reimplemented it from the ground-up. While we haven't tested it, I think that the issue is most likely fixed on that branch. Hence, I'm closing it. Still, we will accept PR fixes until v1-beta takes over the master branch.