plotly / dash-core-components

OBSOLETE: now part of https://github.com/plotly/dash
https://dash.plotly.com
MIT License
271 stars 145 forks source link

Some components don't render className, id, or style #344

Open rmarren1 opened 6 years ago

rmarren1 commented 6 years ago

Doesn't render id

These components do not actually add the id prop to the page

Has className prop that is not used

These components have a className prop, but it does not actually do anything

T4rk1n commented 6 years ago

I think there's a couple more component that are either missing className, id or style, we should take a look over all the components and ensure they all use them.

rmarren1 commented 6 years ago

ConfirmDialog is missing style and className, but is it actually possible to style that alert menu?

T4rk1n commented 6 years ago

no

valentijnnieman commented 6 years ago

The Dropdown uses className and other props like style here, on line 98:

    {...omit(['fireEvent', 'setProps', 'value'], this.props)}

it uses the spread operator to attach all the props except those in the omit

valentijnnieman commented 6 years ago

Same goes for the Input. Are there any other components you've found that might miss any props? Otherwise I think we can close this issue.

T4rk1n commented 6 years ago

@valentijnnieman It gives those props to ReactDropdown which doesn't use it.

T4rk1n commented 6 years ago

I definitely remember having to wrap some components with a div just so I could add an id or className to a component that will actually be used (like in a integration test the component with an id selenium was failing to find because it wasn't actually used in render).

rmarren1 commented 6 years ago

I was trying to make it inline, but I guess you need to set this with the style attribute since there is no className set in the wrapper

valentijnnieman commented 6 years ago

Ah I see, yeah, you're totally right. https://github.com/bvaughn/react-virtualized-select there's a style prop on that component but no className or id or other. We could wrap those components in a div which takes those props I guess, although for className that would not really be what you'd expect. For the Dropdown I guess the only real fix would be to use another dropdown component that is more customizable with CSS, or write one ourselves.

rmarren1 commented 6 years ago

I'll just look through all the components and add what I find to the top level.

valentijnnieman commented 6 years ago

The Tab component should render it's props, including id, look in the EnhancedTab component in Tabs.react.js @rmarren1

rmarren1 commented 6 years ago

@valentijnnieman Oh okay, I see your comment explaining that in Tabs now.