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

Component: Dropdown #6

Closed
patrickarlt opened this issue May 9, 2019 · 6 comments
Closed

Component: Dropdown #6

patrickarlt opened this issue May 9, 2019 · 6 comments

Comments

@patrickarlt
Copy link
Contributor

Esri/calcite-web#1021 and Esri/calcite-web#1017.

Also See: https://ionicframework.com/docs/api/select

@macandcheese
Copy link
Contributor

macandcheese commented Jun 18, 2019

Proposed component structure, looking for feedback, some q's.

<calcite-dropdown>
  <calcite-dropdown-group>
    <span slot="dropdown-header">Group 1</span>
    <calcite-dropdown-item>Foo</calcite-dropdown-item>
    <calcite-dropdown-item active>Baz</calcite-dropdown-item>
    <calcite-dropdown-item>Bar</calcite-dropdown-item>
  </calcite-dropdown-group>
  <calcite-dropdown-group>
    <span slot="dropdown-header">Group 2</span>
    <calcite-dropdown-item>Zip</calcite-dropdown-item>
    <calcite-dropdown-item active>Zap</calcite-dropdown-item>
  </calcite-dropdown-group>
</calcite-dropdown>

<calcite-dropdown>

  • Assign a GUID
  • props: theme=(dark/light)
  • methods: openCalciteDropdown, closeCalciteDropdown
  • events: calciteDropdownOpen, calciteDropdownClose (emit dropdown ID)

<calcite-dropdown-group>

  • Assign a GUID

<calcite-dropdown-item>

  • Assign a GUID
  • attributes: active (should we support multiple active within a single group? Is there a use case for that?)
  • events: calciteDropdownItemSelected / deselected (emit dropdown ID, group ID, item ID (or value??))

@patrickarlt
Copy link
Contributor Author

patrickarlt commented Jun 20, 2019

@macandcheese this looks good to me. I'm assuming this would follow these designs:

Example

I do have 2 questions:

  1. Should this support things not in a group? for example:

    <calcite-dropdown>
        <calcite-dropdown-item>Foo</calcite-dropdown-item>
        <calcite-dropdown-item active>Baz</calcite-dropdown-item>
        <calcite-dropdown-item>Bar</calcite-dropdown-item>
    </calcite-dropdown>

    Here is how we would use this in VTSE:

    Screenshot 2019-06-20 08 48 14

  2. Maybe <calcite-dropdown-group> could just have a label property rather then a slot? Since we probably just want to allow text there rather then tab headers which could be icon + text.

    <calcite-dropdown>
      <calcite-dropdown-group label="Group 1">
        <calcite-dropdown-item>Foo</calcite-dropdown-item>
        <calcite-dropdown-item active>Baz</calcite-dropdown-item>
        <calcite-dropdown-item>Bar</calcite-dropdown-item>
      </calcite-dropdown-group>
      <calcite-dropdown-group label="Group 1">
    
        <calcite-dropdown-item>Zip</calcite-dropdown-item>
        <calcite-dropdown-item active>Zap</calcite-dropdown-item>
      </calcite-dropdown-group>
    </calcite-dropdown>
  3. Maybe a size prop with a possible `compact value?

I can't really see anyone needing multiple active items in a single group but if there is a valid use case I don't see why not.

One last thing (I keep thinking of stuff) it would be useful to customize the appearance of the button via a <slot>:

<calcite-dropdown>
  <button slot="button" class="btn btn-small btn-white">Dropdown Menu</button>
  <calcite-dropdown-group label="Group 1">
    <calcite-dropdown-item>Foo</calcite-dropdown-item>
    <calcite-dropdown-item active>Baz</calcite-dropdown-item>
    <calcite-dropdown-item>Bar</calcite-dropdown-item>
  </calcite-dropdown-group>
  <calcite-dropdown-group label="Group 1">

    <calcite-dropdown-item>Zip</calcite-dropdown-item>
    <calcite-dropdown-item active>Zap</calcite-dropdown-item>
  </calcite-dropdown-group>
</calcite-dropdown>

@macandcheese
Copy link
Contributor

macandcheese commented Jun 20, 2019

I'm assuming this would follow these designs:

Yep! It was actually already added in CW2.0 branch, https://github.com/Esri/calcite-web/blob/2.0/lib/sass/calcite-web/components/_dropdown.scss, just going to port over and add the dark theme variant.

  1. Should this support things not in a group? for example:

Yeah, totally. I had just imagined that being a wrapper in the case where there ARE more than one group - mostly for emitting a containing group ID. I'll make it work without as well.

  1. Maybe <calcite-dropdown-group> could just have a label property rather then a slot? Since we probably just want to allow text there rather then tab headers which could be icon + text.

Makes sense, I'm always in favor of "locking down" as much as possible to prevent folks from putting not... good looking stuff in, ha.

  1. Maybe a size prop with a possible `compact value?

Yeah I think thats useful here, as well in other components potentially. Could be something set globally (and still explicitly override-able) alongside theme - for VTSE or other 'dense' apps. Maybe some adjustments to font-size and the $baseline padding variable could work.

I can't really see anyone needing multiple active items in a single group but if there is a valid use case I don't see why not.

Hm, that would make a better case for using "super multi select" vs. here, where a click would "deselect" all other items in the group. Open to discussion on that.

One last thing (I keep thinking of stuff) it would be useful to customize the appearance of the button via a <slot>:

I think the invoking element is actually outside the scope of dropdown component itself? ie...

<button class="btn" onclick="document.querySelector('#dropdown-one').toggleCalciteDropdown()">
Dropdown Menu
</button>

<calcite-dropdown id='#dropdown-one'>
...
</calcite-dropdown>

@patrickarlt
Copy link
Contributor Author

<button class="btn" onclick="document.querySelector('#dropdown-one').toggleCalciteDropdown()">
Dropdown Menu
</button>

<calcite-dropdown id='#dropdown-one'>
...
</calcite-dropdown>

I have 2 problems with this code snippet:

  1. To which element do we "attach" the dropdown? It isn't 100% clear that we want to attach our dropdown to the <button> element.
  2. This would require an imperative API which I would really like to avoid for components. Creating an imperative API makes integration harder since users have to get a ref to the component and call a method on it. Definitely possible and in most cases easy but it leave more room for error.

@macandcheese
Copy link
Contributor

That's fair - I'll update to use a slot for the invoking element.

@macandcheese
Copy link
Contributor

PR in - #96

@macandcheese macandcheese removed their assignment Aug 13, 2020
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