TiddlyWiki / TiddlyWiki5

A self-contained JavaScript wiki for the browser, Node.js, AWS Lambda etc.
https://tiddlywiki.com/
Other
8.08k stars 1.19k forks source link

[BUG] TW UI uses "tc-reveal" class so exessively, that it has no value for styling anymore #8709

Open pmario opened 3 weeks ago

pmario commented 3 weeks ago

Describe the bug

Talking about "cruft".

The core UI uses tc-reveal so excessively, that it has absolutely no value for styling anymore. The only thing it does is wasting CPU cycles needed for parsing.

Expected behavior

We should remove it.

tc-reveal is not used for styling in any core theme CSS tiddlers. -- (I think for a reason)

To Reproduce

See screenshot below

title: show-tc-reveal
tags: $:/tags/Stylesheet

.tc-reveal {
  outline: 2px solid red;
}

Screenshots

image

TiddlyWiki Configuration

v5.3.5

Additional context

No response

CodaCodr commented 3 weeks ago

Blindly removing it will "break" anyone's wiki that is using tc-reveal.

.something .something-else .tc-reveal .my-thing { ...
pmario commented 3 weeks ago

We did already remove a lot of them when we changed from reveal-widget to <%if%> -- We did not get a single issue yet.

CodaCodr commented 3 weeks ago

Absence of evidence is not evidence of absence.

If you (we) remove it, it MUST be presented as a potentially breaking change.

CodaCodr commented 3 weeks ago

I did a search of a couple of my wikis. Tobi Beer's appear plugin uses it. My code uses it in two places. I can fix those easily enough -- and thanks to bundler, I can propagate it too ;)

I fear there are more plugins out there that might use it, and other folks that may have long-forgotten CSS relying on it.

Either way, "it has no value for styling anymore" is not a true statement.

Jermolene commented 3 weeks ago

Hi @pmario sadly I think this is indeed something that would be problematic to change at this point. Perhaps the best thing we could do is to hasten the deprecation of the reveal widget. I think the current obstacle is that it is the only way to make an animated appearance/disappearance.

ericshulman commented 3 weeks ago

I also have used .tc-reveal in https://tiddlytools.com/#TiddlyTools%2FCatalog, which contains this CSS:

.tt-catalog {
    & .tc-tab-content .tc-reveal>p  { margin:0; }

which eliminates unwanted top/bottom margins for the first <p> element within that specific .tc-reveal

For this particular use-case, I could replace .tc-reveal with >div so that it no longer depends upon the class name, like this:

.tt-catalog {
    & .tc-tab-content >div>p        { margin:0; }

However, this example illustrates that completely removing .tc-reveal from the $reveal widget output can produce visible backward-compatibility issues for some people.

As far as TWCore performance is concerned, I suggest just continuing to replace the use of $reveal widgets with <%if%> whenever possible, but leave the use of .tc-reveal within the $reveal widget implementation for those who are currently using it in their plugins or wiki content.

Leilei332 commented 3 weeks ago

Actually I wonder the difference between the reveal widget and conditional shortcut (which generates a list widget), and which one has a better performance.

Jermolene commented 3 weeks ago

Actually I wonder the difference between the reveal widget and conditional shortcut (which generates a list widget), and which one has a better performance.

I think here we should be more concerned with making the core simpler for people to understand than marginal performance differences. The reveal widget should really be called the "animate" widget because that is the only remaining use case for it. For all other cases the conditional shortcut syntax is more readable and flexible.

ericshulman commented 3 weeks ago

the reveal widget... is the only way to make an animated appearance/disappearance.

Even without animation, the "retain" feature of the reveal widget can be used as an optional parameter in the <<tabs>> macro. This can be important for efficient tab switching, particularly if the tab content has a high initial rendering overhead, but a low occurence of refresh triggers.