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

Allow rowStyle on tables to override default styles? #1171

Closed
emroussel opened this issue Jul 12, 2018 · 3 comments
Closed

Allow rowStyle on tables to override default styles? #1171

emroussel opened this issue Jul 12, 2018 · 3 comments

Comments

@emroussel
Copy link
Contributor

I have a table where I would like to have some content overflow from a row. However, passing a rowStyle object with overflow: 'visible' can't do it, because the Table component merges the rowStyle object with some other styles (including overflow: 'hidden') here.

We use css in js everywhere in the project, so using a className to override this would be a little inconsistent and confusing.

So I was wondering what the reasoning behind forcing default some styles is?

I would suggest changing this

style: {
    ...rowStyleObject,
    height: headerHeight,
    overflow: 'hidden',
    paddingRight: scrollbarWidth,
    width: width,
}

To this

style: {
    height: headerHeight,
    overflow: 'hidden',
    paddingRight: scrollbarWidth,
    width: width,
    ...rowStyleObject,
}

To give users the option to override the styles if they want, while still keeping the defaults if they don't override them.

If you guys agree to this change, I'm more than happy to submit a PR for it!

@wuweiweiwu
Copy link
Contributor

hi, @emroussel

That is because the height is used in the positioning of the cells. Thus if overflow wasn't hidden it would mean that it will cover other cells.

@emroussel
Copy link
Contributor Author

@wuweiweiwu having overflow: visible seemed to work for me, the layout looked the same, and the height was properly set.

Either way, I feel like it would make sense to allow users to override these default styles when passing in a style object since they can do the same with a class.

What do you think?

@wuweiweiwu
Copy link
Contributor

@emroussel I think thats valid. Feel free to open a PR! it'll be much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants