Proposal: Avoid some uses of Reveal Widget

[Alzacon]

I have a "problem" with this widget. It creates messy html, many tags (div and span) with class tc-reveal are created.

It is more evident, for example, in the tiddler toolbar. A filter is used in the template for create a list, and then its items are evaluated in the reveal widget. I don't know if there is a special reason for its use instead of other solutions as filter expression. I found a "supercalifragilisticexpialidocious" filter expression that avoids the use of the widget.

Origal snippet:

(for the part visible of the tiddler toolbar is)

<$list filter="[all[shadows+tiddlers]tag[$:/tags/ViewToolbar]!has[draft.of]]" variable="listItem">
    <$reveal type="nomatch" state=<<config-title>> text="hide">
        <$set name="tv-config-toolbar-class" filter="[<tv-config-toolbar-class>] [<listItem>encodeuricomponent[]addprefix[tc-btn-]]">
            <$transclude tiddler=<<listItem>>/>
        </$set>
    </$reveal>
</$list>

Proposed snippet:

(with 2nd run in filter expression: -[all[shadows+tiddlers]prefix[$:/config/ViewToolbarButtons/Visibility/]field:text[hide]!has[draft.of]removeprefix[$:/config/ViewToolbarButtons/Visibility/]] )

<$list filter="[all[shadows+tiddlers]tag[$:/tags/ViewToolbar]!has[draft.of]] -[all[shadows+tiddlers]prefix[$:/config/ViewToolbarButtons/Visibility/]field:text[hide]!has[draft.of]removeprefix[$:/config/ViewToolbarButtons/Visibility/]]" variable="listItem">
    <$set name="tv-config-toolbar-class" filter="[<tv-config-toolbar-class>] [<listItem>encodeuricomponent[]addprefix[tc-btn-]]">
       <$transclude tiddler=<<listItem>>/>
    </$set>
</$list>

It works, even on TWs with extra buttons.

NOTE: I don't know if there is any better filter expression for it. Although with a new special filter we could try to avoid the long second run of the filter expression (prefix+ field+ removeprefix).

[saqimtiaz]

The filter part of this can be simplified to something like this in 5.1.23:

[all[shadows+tiddlers]tag[$:/tags/ViewToolbar]!has[draft.of]] :filter[lookup:show[$:/config/ViewToolbarButtons/Visibility/]!match[hide]]

Personally I prefer to use the reveal widget only where the resultant DOM node is needed. I don't have the opportunity to look into the toolbar structure at the moment, but we would need to make sure that the change is backwards compatible and would not have any layout or performance implications.

There are a few places in the other templates as well that we can optimize using some of the newer filter run prefixes and avoid the use of extra widgets.

[Alzacon]

It looks much better.

It's true. There are few places where reveal is used, but it creates many nodes.

My snippet works at least from version 5.1.13. The lookup operator was introduced in 5.1.15.

I have no idea about performance.

[Alzacon]

I edited the title because it sounded too aggressive and rough, sometimes i can do it without this intention. Sometimes it happens to me when I think I have things clear.

[Arlen22]

Jeremy said in a discussion once that the list widget is preferred if animation is not needed. If there is animation, I think there should still be a way around this problem, such as actually setting classes on the reveal widget, which is already a feature, instead of putting a div inside it.

https://tiddlywiki.com/#RevealWidget

[Alzacon]

So it would be like a new action-widget?

[Jermolene]

Hi @Alzacon @saqimtiaz @Arlen22 that's correct: firstly, that the purpose of the <$reveal> widget is to generate the DOM nodes necessary for us to animate content appearing/disappearing. And secondly that we overuse the <$reveal> widget within many of the core page templates, leading to many extraneous DOM elements. I'd favour clearing things up where we can. In the specific case of toolbar buttons I'd like to be able to encapsulate the common functionality better, and make it easier to make additional custom toolbars.

[Alzacon]

In the specific case of toolbar buttons I'd like to be able to encapsulate the common functionality better, and make it easier to make additional custom toolbars.
I don't know if I understad it. Would it be like toc macro? A toolbar macro?

We have ViewToolbar, ( EditToolbar,) EditorToolbar and PageControl. Are there more?

I tried a basic macro but it doesn't work because they have different designs. I need rethink how fix the differences.

[Arlen22]

You can see an example of something I had proposed at one time, but got no further.

https://github.com/Jermolene/TiddlyWiki5/pull/3891/files

I believe we have a match operator now though, which I think should be used to save a few CPU cycles.

[pmario]

I'm OK with removing unnecessary DOM elements. ... We only have to take care, that we don't accidentally change the <<qualify ID which is sensible to transclusions. ... Just a reminder.

[Jermolene]

I'm OK with removing unnecessary DOM elements. ... We only have to take care, that we don't accidentally change the <<qualify ID which is sensible to transclusions. ... Just a reminder.

The reveal widget doesn't affect the variables used by the qualify macro, as can be verified like this:

<<qualify "abc">>
<$reveal state="Missing" type="match" text="" default="">
<<qualify "abc">>
</$reveal>

Output:

abc-886000927

abc-886000927

[Alzacon]

I couldn't solve the difference in the design in "basic" toolbars (ViewToolbar, EditToolbar and PageControls) vs EditorToolbar. EditorToolbar uses a template while the other toolbar use the basic transclusion. I didn't found how fix it without use a set widget for each button. Surely I don't have the knowledge to solve it.

I think in two alternative solutions:

• Create a tiddler template for the buttons of "basic" toolbars, with basic transclusion. Maybe it is possible do a template that does extra refactoring.
• Add a transclude widget that the empty string in tiddler attribute, tiddler="", works like currentTiddler variable.

Here a approach of proposal with examples, there are some extra (unused) macros for the part of dropdowns. It is based on contributions of @saqimtiaz and @Arlen22 . The filter expresion only works in 5.1.23.