Widgets: allow custom attributes on DOM nodes

[saqimtiaz]

With the introduction of the <$eventcatcher> widget, and a desire to use attribute selectors for CSS, I am increasingly finding the need to be able to specify custom attributes on the DOM nodes generated by the button, reveal and droppable widgets.

Currently the only workaround is to wrap the widgets in an additional wrapper div/span which leads to a more cluttered DOM structure and potentially negates some of the performance savings to be had from the above approaches.

One potential solution would be that user-defined attributes that are prefixed with data- are used to assign custom attributes. So <$droppable tag="div" data-node-title="mytitle"> would give us <div class="tc-droppable" data-node-title="mytitle">

This is reminiscent of #5155 that discusses allowing user-defined widget attributes that are turned into variables for actions strings.

Also relevant here for a longer term vision might be this brief discussion with @Jermolene about a more declarative syntax for widgets:

If we're dreaming a little, I've always wanted to make the widget interface more declarative. For example, a widget could declare a structure describing the attributes that it requires, and the system could then deal with a lot of housekeeping around rendering and refreshing. With a bit more metadata we could start to think about popup help in the editor as one starts to type a widget naem.

Thoughts?

[Jermolene]

It's a great idea.

Some widgets generate multiple DOM nodes (eg scrollable, but there are others). It may be useful to be able to use different prefixes to identify them so that attributes can be assigned independently.

It's long irritated me that so much of the mapping from widget attributes to DOM attributes is hand wired for each widget. With the declarative approach we could even have the generated DOM nodes specified as an object tree that would allow these mappings to be expressed. The tree would be automatically instantiated and the output nodes passed to the render/refresh function for it to do any further customisation as required.

[saqimtiaz]

"hand wired" referred to the code that ties widget attributes to DOM nodes

@pmario this is the key bit. For example how the class attribute on a widget assigns the className to a DOM node.

[Jermolene]

Another quick thought: if we're upgrading the widget implementation, we should consider adding a destroy() method so that widgets can clean up after themselves. The current implementation is rather an invitation to cause memory leaks.

Perhaps we should start a ticket to bring together all of these proposed improvements under the banner "Simplify implementation of widgets". Our north star should be to try to make writing widgets as accessible to newcomers as writing JS macros.

[saqimtiaz]

@Jermolene every time I look at the widget implementations and base class I am surprised memory leaks are not an issue!

I can pull together a ticket this weekend. We will have to decide if we make the new implementation backwards compatible or we consider a breaking change, maybe for 5.2

[Jermolene]

@Jermolene every time I look at the widget implementations and base class I am surprised memory leaks are not an issue!

I can pull together a ticket this weekend. We will have to decide if we make the new implementation backwards compatible or we consider a breaking change, maybe for 5.2

Thank you @saqimtiaz. I was thinking that this could be done in a backwards compatible way with a bit of care. It could be a matter of introducing a new base class derived from our existing widget base class with the declarative sugar support, and then migrating our widgets to inherit from that instead of the current class. Existing widgets based on the existing base class would continue to work.

[Jermolene]

And of course we'd need a new or superwidget or widget2 module type.

[pmario]

<$droppable tag="div" data-node-title="mytitle"> would give us <div class="tc-droppable" data-node-title="mytitle">

I think that's a good idea, since data- prefixed elements already has native browser API support.

I would like to have a "should" convention for "core-names" like eg: data-twMyTitle or data-tw-my-title. It will be accessible as domNode.dataset.twMyTitle or . `domNode.dataset["tw-my-title"]. ...

So plugin authors are able to use their own prefixes. eg: I would use: wltw-my-title if it is specific for my plugin. I'd use tw-new-var if I'd like to extend the core.

[pmario]

@Jermolene @saqimtiaz Just a reminder.

I've landed here again and at "[IDEA] widgets, that allow "actions" should be able to pass on "user defined" variables and attributes" #5155 with the problem that the <$keyboard widget isn't capable to pass on variables to the actions it activates.

[saqimtiaz]

@pmario thanks for the reminder. I've had a few busy weeks at work and need to catch up on a few other things first, after which I'll take another look at how we might implement this and #5155 in an elegant and backwards compatible manner.

[BurningTreeC]

In my PR #5695 I'm surrounding the div of the tc-tiddler-frames with 4 keyboard widgets, where the first one needs to create a div in order for the storyview-animations to work correctly. Now I also found, that my muuri storyview plugin (https://github.com/BurningTreeC/tiddlywiki-muuri) needs that first div to be of class tc-tiddler-frame.

The optimal solution would be to set the classes (tc-tiddler-frame etc.) on that first div created by the keyboard widget and all data-attributes (and attributes like tabindex), too.

[BurningTreeC]

What would it take to get a PR out for this? I'd like to give it a try, but I'd need some advice

[saqimtiaz]

A few thoughts off the top of my head:

I can imagine this implemented as a method in the widget base class, that when called iterates through the widget attributes (perhaps only those prefixed with dom- ?) adding the specified attributes to the DOM node. So for example, dom-tabIndex specifies the tabIndex attribute.

A prefix seems necessary so that widgets can be extended with new attributes in the future without clashing with an attribute name someone may have used. However, is the dom- prefix intuitive enough for end users? It may not matter if this is considered an advanced use case.

We will need to decide which attributes should be supported and filter for invalid attribute names. In this thread we have discussed adding data- attributes to make it easier to work <$eventcatcher/>. Adding classes and attributes such as tabIndex are equally valid use cases. Furthermore, some widgets already support class and tabIndex attributes so the mechanism would need to play well with that.

Also, some widgets such as <$scrollable/> create multiple DOM nodes. So there would need to be an affordance for specifying to which DOM node an attribute should be applied.

The tricky bit with this is getting the logic and approach right, the code itself should not be tricky. I haven't had a chance to sit down and give this some proper thought yet, but hopefully this helps.

[BurningTreeC]

Thank you @saqimtiaz, I was also thinking about a method in the widget base class

The logic and approach though are something I'm hoping to elaborate here

[BurningTreeC]

Hi @saqimtiaz,

Have you got any more ideas for this project here?

[saqimtiaz]

@BurningTreeC I personally think that is this is best implemented at the same time as the new declarative approach for widgets. Otherwise we will be more constrained by backwards compatibility for that change. As such I would like for us to take our time and address this after the 5.2.0 release.