markusenglund / react-switch

A draggable toggle-switch component for React. Check out the demo at:
https://react-switch.netlify.com/
MIT License
1.33k stars 100 forks source link

Can you add title please #13

Closed tarim closed 6 years ago

tarim commented 6 years ago

Hi there,

When I added the html title to the switch, and it didn't work. Then I checked, the title attribute is missing. Can you add it, please?

Thank you,

markusenglund commented 6 years ago

Hi Tarim,

What do you think about putting the title on a surrounding element instead:

<span title="Cool title" style={{ display: "inline-block" }}>
  <Switch
    onChange={this.handleChange}
    checked={this.state.checked}
    className="react-switch"
    id="normal-switch"
  />
</span>

Using the title attribute is not all that common, and I would rather not make the API of this library even bigger unless absolutely necessary.

tarim commented 6 years ago

Hi @markusenglund,

For the short term, it works. Thank you. But, the long-term, I think it should have to include all the HTML attributes.

Thank you,

Nur

markusenglund commented 6 years ago

All html attributes have to be added individually (rest-spread won't work) and documented. There are a lot of attributes and they exist on a spectrum from most useful to least useful. The goal is to support all of the ones that are at least somewhat useful to some people. I'm not sure title qualifies since it's not that common and can be worked around with a wrapping element.

chiefGui commented 6 years ago

@tarim just an idea:

export const Switcher = ({ style, switchProps, ...rest }) => (
  <span style={{ display: "inline-block", ...style }} {...rest}>
    <Switch {...switchProps} />
  </span>
)
import { Switcher } from './ui'

const App = () => (
  <div>
    <h1>Hello world!</h1>

    <Switcher title="Something something" switchProps={{ onChange: doSomething }} />
  </div>
)

const doSomething = () => console.log('Hello world!')

(Just have written the code above from the top of my head. Might require some adjusts.)

tarim commented 6 years ago

Hey @chiefGui,

That is better than add span tag all the time. I am using it with the bootstrap buttons, and I have a hard time with the style. It never lines up with buttons. image

chiefGui commented 6 years ago

I have the feeling that's happening because Bootstrap is adding a margin-top of 5px in your buttons group. If you add a margin-top: 5px; in the wrapper of your Switch, maybe that will align it properly.

There's one gotcha, though: the Switch doesn't have the same height as the button, meaning that regardless of your margin-top: 5px; it won't center vertically against the buttons.

One solution that might work for you is using flexbox. Quick example:

<div style={{ display: 'flex', alignItems: 'center' }} class="buttons">
  <button class="btn">Something</button>
  <button class="btn">Something</button>
  <Switcher />
</div>

This way all the elements of your container div will be vertically centered, the Switch included. Give it a shot.

Note: let's keep this issue clean and avoid making it go off-topic. If you want further assistance with flexbox and centering things using it, please refer to this awesome cheatsheet.

tarim commented 6 years ago

@chiefGui ,

I tried margin-top before and it didn't work at all. Thank you for the tips 'flex'. It works with a little bit of extra style. such as `.react-switch-bg{ margin: 3px 0 0 0 !important; }

.react-switch-handle{ margin-top: 3px; }`