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

Honor tabindex if set, save it when disabling a component. #51

Closed depeele closed 6 years ago

depeele commented 6 years ago

Add _updateAccessabilityAttributes() to components that use tabindex for the purpose of managing accessability attributes, including tabindex:

jarek-foksa commented 6 years ago

Is there any reason why you are storing the previous tabindex as an attribute:

this.setAttribute('data-tabindex', tabIndex);

... rather than as a JS object property (where $tabIndex is a Symbol):

this[$tabIndex] = tabIndex;?

The only drawback of the latter approach I can think of is that the old tabindex might get lost when using virtual DOM.

depeele commented 6 years ago

The only reason was to try and keep attributes associated with DOM elements. If the DOM tree is cloned for any reason, the data-tabindex goes along automatically.

It could work just as well as a property of the instance, using a $tabIndex symbol...

I just ran across the need to honor the tabindex that was set on the element and approached it this way...

The "right" approach is really your call.

jarek-foksa commented 6 years ago

I have refactored your code to use symbols. Let me know in case you notice any issues.