TiddlyWiki / TiddlyWiki5

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

[BUG] Dynannotate refresh bug (?) #7257

Open yaisog opened 1 year ago

yaisog commented 1 year ago

Describe the bug

Playing around a bit more with Dynannotate and my favorite new widget, $genesis, I noticed an anomaly.

When wrapping the body view template in $:/core/ui/ViewTemplate/body with a conditional $dynannotate (courtesy of $genesis), which is controlled by a temporary tiddler containing the search term, like so:

<$let searchTerm={{{ [<currentTiddler>addprefix[$:/temp/dynannotate/]get[text]] }}} genesisType={{{ [<searchTerm>!is[blank]then[$dynannotate]] }}} >
    <$genesis $type=<<genesisType>> search=<<searchTerm>> searchDisplay="overlay" searchMode="normal" searchCaseSensitive="no">
        <$transclude tiddler={{{ [<currentTiddler>] :cascade[all[shadows+tiddlers]tag[$:/tags/ViewTemplateBodyFilter]!is[draft]get[text]] :and[!is[blank]else[$:/core/ui/ViewTemplate/body/default]] }}} />
    </$genesis>
</$let>

where only the $let and $genesis lines were added, navigating to the tiddler with zoomin storyview does not highlight the terms until some other action is performed, i.e. another refresh is triggered.

Checking the DOM, the containers for $dynannotate are there, but empty until the refresh.

Update 2023-02-14: When used in modals, similar behavior can be observed, as explained in more detail in this comment of the current issue.

Expected behavior

The highlight should appear immediately after navigating to the tiddler.

To Reproduce

Go to tiddlywiki.com/prerelease. Activate zoomin storyview. Modify $:/core/ui/ViewTemplate/body as described above. Create a tiddler $:/temp/dynannotate/HelloThere with the text "to". Navigate to HelloThere. Click a button or sidebar tab.

Screenshots

No response

TiddlyWiki Configuration

5.2.6-prerelease

Additional context

No response

yaisog commented 1 year ago

Also, using $dynannotate anywhere in HelloThere causes this: image In new tiddlers, Dynannotate works OK.

The bug described above has nothing to do with this. It "works" on new tiddlers as well. I was just checking if I could reproduce the bug if $dynannotate was not placed in the body template, but the tiddler itself, and I used HelloThere because it was right there.

Jermolene commented 1 year ago

Thanks @yaisog these both need investigation but I think the RSOE needs to be a separate ticket.

yaisog commented 1 year ago

I was afraid you're gonna say that. 😢 Coming up.

Is there a prize for most issues submitted in any given week?

yaisog commented 1 year ago

After a bit of debugging, at can at least say that the dynannotate code runs while the previous tiddler is still shown in zoomin view. It does match the correct text and finds the right matches, and cannot create the overlays because the getClientRects() function in createOverlay returns nothing. With the next refresh, the overlays are created and shown correctly. It makes no difference if the target tiddler was previously hidden (as in my wiki) or not (as on tiddlywiki.com).

I'm not sure if it's possible (or desirable) to delay creating the overlays until the child content of $dynannotate has finished rendering...

yaisog commented 1 year ago

In the code in my OP, I might circumvent the problem by setting the temporary tiddler which controls the overlay after navigation to the target tiddler that will receive the overlay, forcing a refresh after it has been rendered.

Maybe this case is edgey enough to live with it (for now)...?

Jermolene commented 1 year ago

Thanks @yaisog this still seems a bit curious. The dynannotate widget is only concerned with its child DOM nodes, so it's hard to see how zoomin's shenanigans can affect that. I'm happy if you can work around it, but would be interested to get to the bottom of this.

yaisog commented 1 year ago

zoomin's shenanigans

😄 You know that zoomin is the best storyview, right? The. best. Well, at least for large wikis. And for newcomers who might not be used to the vertical content concatenation. I'm kidding. It's totally subjective. Also, it's great that there are different options...

It turns out that since I'm starting in a (search) modal from which I control the highlighting of the search term in the subsequently displayed result, I get a refresh anyway when the modal is closed. So, it isn't really a problem for me. But I can relate to you wanting to get to the bottom of this...

Jermolene commented 1 year ago

The zoomin story view is regularly abused. It wasn't designed as a single tiddler view, nor is it particularly good at it. Its purpose is to support the animation that bears its name.

yaisog commented 1 year ago

It wasn't designed as a single tiddler view, nor is it particularly good at it.

I know, but it's the only one we have. I actually recently derived a single storyview from zoomin, with all the animations taken out, see https://talk.tiddlywiki.org/t/zoomin-storyview-without-navigation-animations/6106. There wasn't particularly much attention paid by the community for that post. I'm guessing that my fondness for zoomin puts me in a small minority.

If I ever find the time, I'd like to integrate suppression of any refreshing of the hidden tiddlers until they are shown again. Currently, I'm doing it via folding / unfolding upon intercepting tm-navigation messages, but I'd like to have it in the storyview itself. How would you go about it? Just set the folded state tiddler to hide whenever display: none; is set (I've already changed all ViewTemplates to set retain="no" in the corresponding $reveal)?

Jermolene commented 1 year ago

Hi @yaisog the trouble is that it is not possible to create a decent single tiddler view using a storyview, because the storyview has no way to limit the number of entries displayed by the list widget. So the most recent experiments I have done on this take a different approach: in TWPub we re-implement the navigator widget as a custom wikitext widget, and have a new, wiki-text based architecture for storyviews. Here is the classic storyview:

https://github.com/TWPUB/TWPUB-Tools/blob/main/plugins/twpub-tools/%24/core/ui/storyrivers/classic.tid

And here's the single tiddler view:

https://github.com/TWPUB/TWPUB-Tools/blob/main/plugins/twpub-tools/%24/core/ui/storyrivers/single-tiddler.tid

Once we've merged #6666 then I intend to start looking at folding the TWPub work back into the core.

yaisog commented 1 year ago

the trouble is that it is not possible to create a decent single tiddler view using a storyview, because the storyview has no way to limit the number of entries displayed by the list widget.

That being said, one could change the list filter in $:/core/ui/PageTemplate/story from filter="[list[[$:/StoryList]" to filter="[{$:/HistoryList!!current-tiddler}]" to show only one tiddler. This couldn't be done in the storyview UI setting and would be (yet another) core override, but possible?

re-implement the navigator widget as a custom wikitext widget

You really are rewriting the core in WikiText, aren't you? 😲 In your redefinition, it looks like the list of tiddlers in the storyview can be configured via a corresponding field, a bit like the filter in the story PageTemplate above. Sadly, PR #6666 is not here, yet.

Jermolene commented 1 year ago

That being said, one could change the list filter in $:/core/ui/PageTemplate/story from filter="[list[[$:/StoryList]" to filter="[{$:/HistoryList!!current-tiddler}]" to show only one tiddler. This couldn't be done in the storyview UI setting and would be (yet another) core override, but possible?

Right, that works better with the classic storyview than the zoomin storyview.

re-implement the navigator widget as a custom wikitext widget

You really are rewriting the core in WikiText, aren't you? 😲 In your redefinition, it looks like the list of tiddlers in the storyview can be configured via a corresponding field, a bit like the filter in the story PageTemplate above. Sadly, PR #6666 is not here, yet.

I'm keen to merge it as soon as possible.

yaisog commented 1 year ago

I noticed similar behavior with modals that is likely related to the OP bug:

When using $dynannotate in a modal, the annotations do not appear until another refresh happens (because a button is pressed or some such). An example with importable tiddlers is given here: https://talk.tiddlywiki.org/t/does-th-page-refreshed-fire-before-modals-are-updated/6026/18.

Jeremy made this comment:

Very weirdly, in my experiments, I found that if the focus is in the sidebar search box when the modal button is clicked then the highlighting is displayed correctly, otherwise it is not. Strange, because there’s no obvious link to the fresh mechanism…

I have added a note pertaining to the additional information in this post to the OP.