Does th-page-refreshed fire before modals are updated?

Developers
#5

Whoa, it’s one thing to write a little filter operator that’ll hurt noone, but adding hooks to the innermost innards of the clockwork that is TW? Well, at your own peril…

After some debugging, I think I would add this line in $:/core/modules/utils/dom/modal.js:

refreshHandler = function(changes) {
		headerWidgetNode.refresh(changes,modalHeader,null);
		bodyWidgetNode.refresh(changes,modalBody,null);
		footerWidgetNode.refresh(changes,modalFooterButtons,null);
		$tw.hooks.invokeHook("th-modal-refreshed"); /* this line */
};

Close?

#6

In terms of this hook, the context is that we already have quite a few JS hooks in the core; it’s an important way for us to accommodate the needs of JS plugin authors with minimal core impact. There’s an obvious precedent with the existing refresh hooks for the main page template.

In terms of the random operator, the current status is not that the idea is rejected but that there is an ongoing discussion. I’m open to refutations of my concerns. Otherwise, I am hoping that we can figure out a coherent approach for accommodating these non-deterministic filters.

#7

Yes that looks reasonable.

#8

Excellent. The filter is out there as a plugin for people to play with. Maybe we can get some feedback related to your concerns, which I can relate to by the way. That’s why I put a big ol’ notice and corresponding examples for sensible use in its docs. Let’s see what people do with it. Maybe one does not always have to forbid everything that is potentially annoying. Though there will always be ones that don’t read the docs and then complain… :disappointed:

Have a nice day
Yaisog

#9

Is it a problem that this hook will not fire when the modal is initially displayed, but only on its refreshes. It is not for my purpose and it corresponds to its name (th-modal-refreshed), but maybe you want to cover all the bases…?

#10

I think you are right. … But … I also think it depends a bit on what you want to achieve with the hook.

The change event is fired whenever any tiddler in the store changes, which then activates the refreshHandler() function. … So the position you added the hook will be executed even if nothing has been changed in the modal. …

If that’s what you need then OK if not I think you should be more specific, what your hook should do, so we can see if that’s the right place to go.

#11

Well, it doesn’t matter much in my usage (I run mark.js over the contents). The way my modal is set up, any actions change its contents (it’s a search application), so that’ll be OK.

Of course a more generally useful implementation would be preferrable. How is th-page-refreshed hook implemented? Does it only fire when something has actually changed? Then again, there are probably not a lot of things one can do with TW that do not change the page in some way…

How would you do the check whether something has changed? Does the refresh function return something usable?

#12

There is a th-page-refreshing hook alongside the th-page-refreshed hook, so one could make a case for a th-modal-refreshing hook, but in general we don’t add hooks unless they are asked for, and in this particular case it doesn’t appear to be hugely useful.

I had a look at mark.js and can see that it works by mutating the DOM nodes that it runs over.

I’m sure you’ll already be aware, but there are risks with that approach. In general, widgets do not expect other agencies to modify the DOM nodes that they create. No doubt it will work much of the time, but it will be subject to unexpected failures with widgets that make unexpected assumptions about their DOM nodes.

There is already a TiddlyWiki plugin “Dynannotate” in the core library to handle highlighting. The highlighting is implemented as a widget that renders its content into a child div, and then overlays that with another DIV to contain the highlights. That way there is no interference, and there are more options available for how to render the highlights.

The Dynannotate plugin is included in the prerelease:

https://tiddlywiki.com/prerelease/#%24%3A%2Fplugins%2Ftiddlywiki%2Fdynannotate

#13

Hi Jeremy,
thanks, this information is really helpful.

I actually also implemented the th-modal-refreshing hook, since I usually do turn off mark.js with th-page-refreshing and then re-enable it with th-page-refreshed / th-modal-refreshed. This is “old code” on my end and I don’t remember exactly why, only that it didn’t work otherwise. I’m quite sure that your remark about DOM mutation is the explanation.

I will take a look at Dynanotate and see if that can serve my needs. If so, my request for the modal hooks might not be warranted anymore. In this case you might want to not merge the PR, unless someone else has some use for these hooks. I’ll let you know.

Have a nice day
Yaisog

#14

Hi @Yaisog great. If you’re not planning to use the hooks then I’d be inclined not to merge them.

#15

Hi @jeremyruston, after playing around a bit with Dynannotate for about an hour, I stumbled over a couple of things where it differs from mark.js and maybe could be made even better?

  • search mode results in a number of zero-width hits in the text. These are not problematic until you apply a CSS box-shadow to soften the highlight edges. Suddenly, you have soft edges without highlights. Mostly when a line ends with the search text, there is a zero-width annotation at the end of the line, which is inconspicuous as its shadow overlaps the “real” shadow. But then, the next line starts with a zero-width highlight whose shadow is irritating. Here is the underlying HTML with Dynannotate overlay highlights on “Test” (I need to clean up the empty spans):
<button class="tc-btn-invisible">
  <div class="mwi-goto-result-line" data-result-line="7">
    <span class="mwi-goto-journal"></span>
    EA_xxxxxxx › EMV-Test
    <span class="mwi-goto-result-tags"></span>
  </div>
</button>
<button class="tc-btn-invisible">
  <div class="mwi-goto-result-line" data-result-line="8">
    <span class="mwi-goto-journal"></span>
    EA_xxxxxxx › Test Plan
    <span class="mwi-goto-result-tags"></span>
  </div>
</button>
  • Since the highlight is not created by splicing a mark into the text, the transparency of the overlay will change the text color – at the extreme end, setting the background-opacity of the overlay to 1 will result in the text not being visible. I countered this with the CSS mix-blend-mode of the overlay set to darken, which worked for yellow highlights and near-black text. Something like multiply might be more universal, I guess.
  • mark.js can operate on any element identifyable by a CSS selector, Dynannotate must be explicitly added around content. There’s nothing that can be done about that, and it’s OK. I’ll just have to put the widgets everywhere I need them.

Anyhow, this goes a long way towards making modal hooks superfluous.

#16

Yeah, turns out that removing the empty spans solves the problem and removes the zero-width results. Hooray. But not everyone is going to chase down his/her empty spans the way they should.
Anyway, seems to be an edge case on the waaaay outer edge…

#17

Hi @Yaisog I’m not sure I quite understand the empty span problem. Is the issue that dynannotate mistakenly matches empty spans? It would be useful to be able to replicate the problem.

#18

@jeremyruston: OK, I tried to replicate it on https://tiddlywiki.com/prerelease/ but couldn’t. However, what I noticed during testing is that the search overlay didn’t appear at all in a modal, but only when the modal tiddler is shown in the story. The problem I described above happened in a modal, though, so at least in my wiki Dynannotate does work in modals…

You can check it out by importing these tiddlers into the prerelease:
tiddlers.json (2.4 KB)
an opening the demo modal from the Modals tiddler.

#19

Hi @Yaisog the dynanotate plugin doesn’t currently support modals.

The trouble is that modals are currently implemented as an independent rendering context. They should have been implemented along the same lines as alerts, where the modals would be a layer of the main rendering.

We could extend the dynannotate plugin to cover modals, but I suspect that it would be easier and cleaner to move modals into the main rendering.

#20

Hmm, then Dynannotate can’t replace my need for new modal hooks and I need to reopen the PR…

BUT, my tests with Dynannotate and the things I described above were done in a modal, i.e. I do have it working in a modal, even though it shouldn’t (which I didn’t know).

Maybe it has something to do with the modal (a search interface) changing temp tiddlers, which in turn update the modal content, which is inside the $dynannotate?

Here’s the (maybe) relevant code from the modal tiddler:

<$let modal-tiddler=<<currentTiddler>> >
		<$reveal type="nomatch" text="" default={{{ [<resultStore>has[text]] }}} tag="div" class="mwi-goto-results-reveal">
			<$dynannotate search={{{ [<resultStore>get[goto-search-term]] }}} searchDisplay="overlay" searchCaseSensitive="no">
			<$eventcatcher tag="div" class="mwi-goto-results" $pointerover="<<hover-actions>>" selector=".mwi-goto-result-line">
				<$list filter="[<resultStore>get[text]enlist-input[]rest[1]]" counter="counter" emptyMessage="keine Ergebnisse">
					<$button class="tc-btn-invisible" actions="<<click-actions>>">
						<div class="mwi-goto-result-line" data-result-line=<<counter>> >
							<<displayResult>>

I didn’t put the closing tags in the snippet above. resultStore contains the name of a temp tiddler which has a text field with the search results and some other fields.
Dynannotate works in this modal. When I re-open the modal, the contents of the resultStore-Tiddler is initially shown, if it contains results from a previous search. Dynannotate also works in this case (as opposed to only after a change in the resultStore).

I like that it works, please don’t break it. :wink:

PS: I imported the Dynannotate plugin into a TW 5.2.5.

#21

Indeed, you’re quite right, the Dynannotate plugin does work in modals and new windows (I was confusing myself with the Dynaview plugin which does have those restrictions).

#22

@jeremyruston: I have to say, the $dynannotate search highlight functionality is pretty sweet. It’s so much easier to integrate than mark.js was, where I had to keep track of all the active marks via a DOM window property…
:heart_eyes:

For anyone looking to do the same, I found this combination with the equally great $genesis widget pretty useful to conditionally wrap text (e.g. the body ViewTemplate) in a $dynannotate only if a search term has been set (via a state tiddler):

<$let dynannotateText={{{ [<currentTiddler>addprefix[$:/temp/dynannotate/]get[text]] }}}>
    <$genesis $type={{{ [<dynannotateText>!is[blank]then[$dynannotate]] }}} search=<<dynannotateText>> searchDisplay="overlay" searchMode="words" searchCaseSensitive="no">
        content to apply highlighting to goes here
    </$genesis>
</$let>

One would then use e.g. the inputActions of an $edit-text widget to set the text of the state tiddler to the text within the input, thus making $genesis generate a $dynannotate which in turn highlights instances of the input text within its content.
If there is no input, $dynannotate is not even generated.

Caveat emptor: This only works with $genesis from 5.2.6-prerelease onward. Older versions will throw an RSoE when the $type attribute is empty, instead of just rendering the content. Also, searchMode “words” will (hopefully) be added to Dynannotate with PR #7260 - until then use “normal”.

Have a nice day
Yaisog

PS: This has drifted so far from the OP that maybe someone should split this off…

1 Like
#23

Thanks @Yaisog. It would be well worth updating the demo that is included with the Dynannotate plugin to use the same technique, if you’d be able to make a PR?

#24

Hi @jeremyruston, as much as I’d like to take home the PR crown this week, I’d just add this to PR #7260, if that’s alright with you…

Update: Done and done. Take a look – I changed the View Template example quite considerably…