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

filter tweaks #107

Open
freeman-lab opened this issue Nov 14, 2021 · 5 comments
Open

filter tweaks #107

freeman-lab opened this issue Nov 14, 2021 · 5 comments

Comments

@freeman-lab
Copy link
Member

Encountered two challenges while using the Filter component that I wanted to flag. Should be easy fixes.

  1. The label element makes a strong layout assumption (fixed margin of 3). There are at least two places now where the design needs something different. Showing three examples, the first one being the one I'm working on now. Not sure if the answer is to create some options here, or just realize that we'll often need to do something manual (which is what I'm doing now). In the current case, all I wanted was to adjust the margin, whereas use case on CDR Database requires a totally different layout.

CleanShot 2021-11-14 at 11 06 23@2x

CleanShot 2021-11-14 at 11 05 57@2x

CleanShot 2021-11-14 at 11 06 33@2x

  1. We need a way to specify order. Currently it uses whatever order Object.keys returns, which in the present use case is not the intended order. Maybe just an order prop with an array of keys?

cc @katamartin

@katamartin
Copy link
Member

  1. Interesting! My initial thought, especially looking at the second example, is that we might want to capture this label pattern in a component and available for use with arbitrary inputs (e.g. a Field component that wires together the label and input or just a Label component that captures the text styles). This seems preferable to overloading the label prop(s) in the Filter component to handle all possible use cases. In the absence of some type of new label component, I think the manual approach is probably fine for now.
  2. An array of keys to specify order sounds reasonable to me! Another thing that I've stumbled over is that the key dictates the rendered label for the option. I wonder if we could use an interface like:
options (required): Array<string>
values (required): {[option]: boolean} 
labels (optional): {[option]: string} 

The only potential downside would be requiring two props where we used to just require one, but this should prevent any accidental reordering from solely specifying options with an object.

@freeman-lab
Copy link
Member Author

Great thoughts. I'm good to leave (1) out of here for now, can consider one or both of those ideas separately.

I both like the idea in (2) and agree on the downside! The other issue is that having a label and labels prop that control different things is quite confusing.

I'd be good with what you propose if we rename the current label to title, or maybe heading, but I think I prefer title.

Then the props are options, values, labels, and colors, and all are objects keyed by the keys in options. That feels pretty clean to me!

@katamartin
Copy link
Member

katamartin commented Nov 16, 2021

That sounds reasonable to me! Just a flag that this represents a possible transition from

<Filter
  title='Time periods'
  options={['0', '1', '2', '3']}
  labels={{0: '2013-2014', 1: '2015-2017', 2: '2018', 3: '2019': }}
  values={values}
  setValues={setValues}
/>

to something like

<Label>
  Time periods
  <Filter
    options={['0', '1', '2', '3']}
    labels={{0: '2013-2014', 1: '2015-2017', 2: '2018', 3: '2019': }}
    values={values}
    setValues={setValues}
  />
</Label>

(drawing inspiration from theme-ui Label)

@freeman-lab
Copy link
Member Author

why does the <Label> live outside the filter here? shouldn't it be

<Label>Time periods</Label>
<Filter
  options={['0', '1', '2', '3']}
  labels={{0: '2013-2014', 1: '2015-2017', 2: '2018', 3: '2019': }}
  values={values}
  setValues={setValues}
/>

@freeman-lab
Copy link
Member Author

otherwise yep, i'm liking it!

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

No branches or pull requests

2 participants