DesignRevision / shards-react

⚛️A beautiful and modern React design system.
https://designrevision.com/downloads/shards-react/
MIT License
757 stars 97 forks source link

Documentation on Slider is Weak Compared to Other Components (and Other Slider Issues) #22

Open machineghost opened 4 years ago

machineghost commented 4 years ago

Most of the other components are straightforward, and while their documentation is minimal, it's enough. But Slider ... what the heck is the connect prop? It's in literally every example, but you never once even suggest what it's supposed to do ... it just says Array[Bool]Bool.

How does pips work? You've got two examples but never even describe how the prop is supposed to work. I wanted something extremely basic: a slider with only three values. That should have been trivial, but now I'm wondering if I should switch UI libraries (or at the very least go look for a secondary one with a good slider), because I can't even tell whether it's possible or not with this slider, let alone how to actually do it.

The component is full of props, but none are documented :(

Also, when I try to use the useState hook (the official Facebook-recommended way to do state in React in 2019) I get:

index.js:1375 Warning: State updates from the useState() and useReducer() Hooks don't support the second callback argument. To execute a side effect after rendering, declare it in the component body with useEffect().

For code like:

   const [value, setter] = useState([0]);
   return  <Slider
      connect
      onSlide={setter}
      pips={{ mode: "steps", stepped: true, density: 1 }}
      start={value} />

I believe onSlide passses some second argument, which annoys React. This makes everyone have to do:

      onSlide={value => setter(value)}

... except, for some odd reason the component doesn't parseFloat the value (which makes no sense since start is clearly meant to be an array of numbers), so what you actually have to do is:

      onSlide={([value]) => setter([parseFloat(value)])}

... and that only works for single values: it gets uglier if you have multiple (although honestly I still don't "grok" how a two-dimensional slider can even have multiple values ... again with the poor documentation.)

Clearly it'd be simpler if the library let you do:

      onSlide={setter}

or:

      stateSetter={setter}

:)

Also, have you noticed that the slider handle is virtually invisible until you focus it? If the user's monitor doesn't have perfectly calibrated brightness/contrast, they might not even see the handle at all, and not recognize the component as a slider UI.

Please consider making this element more visible: it probably looks perfect on your well-tuned display, but something about the design just fails without perfect contrast, and web developers can't guarantee their users' monitor settings.

machineghost commented 4 years ago

P.S. The closest I've managed to get to a working 3-state slider is:

<Slider
      connect // what does this do?
      onSlide={([value]) => setImportance([parseFloat(value)])} // ugly
      pips={{ mode: "steps", stepped: true, density: 1 }} // no understanding of this
      start={importance} // works great!
      range={{ min: -1, max: 1 }} /// this is not limiting only -1, 0, and 1 as I'd expect
    />
machineghost commented 4 years ago

I figured it out! I had to go read a whole other library's documentation, then read the source code to this library (to discover that I can pass an undocumented format prop ... yes, the Slider has props which are even less documented than the poorly documented ones ;) ), and then I finally got to:

import wNumb from 'wnumb';

(In addition to a "secret" prop, it seems you also need to install and import another library just to tell the Slider "I only want you incrementing/decrementing by 1",)

    <Slider
          connect
          format={wNumb({decimals: 0})}
          onSlide={([value]) => setImportance([parseFloat(value)])}
          pips={{ mode: 'range', density: 0.1 }}
          range={{ min: -1, '50%': 0, max: 1 }}
          start={importance}
          step={1}
        />

If anyone is following in my footsteps maybe the above code will help them ... but again, hopefully this can be improved.

s-kris commented 4 years ago

@machineghost thank you