search

jarek-foksa / xel

Xel - Widget toolkit for building native-like Electron and Web apps
https://xel-toolkit.org
Other
679 stars 58 forks source link

Add a 'checkable' attribute to x-menuitem #59

Closed depeele closed 6 years ago

depeele commented 6 years ago

This is an alternative for pull request #58

Add a checkable attribute to <x-menuitem> to support its use within <x-select>.

Update <x-menuitem> and <x-select> to treat the selected attribute as a boolean attribute.

Update x-select._onMutation() to check for a 'childList' record with an 'x-menu' target. In this case, ensure that all 'x-menuitem' children of the target include a checkable attribute to enable the checkmark functionality of <x-menuitem> required by <x-select>;

jarek-foksa commented 6 years ago

<x-button> has togglable and toggled boolean attributes. <x-checkbox>, <x-radio> and <x-switch> implement toggled boolean attribute. Maybe we should use the same names here?

I have quite a lot of code relying on the current implementation, so I won't be able to merge this pull request until next week.

depeele commented 6 years ago

Makes sense.

How about I change checkable to togglable for starters since that only impacts <x-select> and related styling.

For <x-menuitem>, selected seems more appropriate for a menu item than toggled.

Since this seems to primarily be an issue when using an <x-menuitem> within <x-select>, perhaps use togglable and toggled for that specific use-case?

jarek-foksa commented 6 years ago

Menu items with checkmarks are not meant to be used exclusively inside <x-select>. Material Design guidelines allow them inside e.g. menubars: https://storage.googleapis.com/material-design/publish/material_v_12/assets/0Bx4BSt6jniD7dlkwTUdncmtTTXM/components-menus-usage7.png

I think using "toggled" and "togglable" names everywhere would be less confusing, but I'm not completely sure about it. I will have to research how other widget toolkits are doing it.

jarek-foksa commented 6 years ago

BTW, when <x-menuitem> has togglable attribute then we should automatically change toggled state when user clicks it (as long as clickEvent.defaultPrevented is false).

jarek-foksa commented 6 years ago

I have replaced the tristate selected attribute with boolean toggled and togglable attributes in commit https://github.com/jarek-foksa/xel/commit/06c77a975c57c329e2018acb96d8bcad463b8d48.

Menu items with togglable attribute are now toggled automatically when clicked. You can prevent this behavior by listening for "toggle" event and calling event.preventDefault().