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

Developers
#1

While debugging I think I found that th-page-refreshed fired before a modal (with a text input that caused the refresh) was updated. More explicitly, the edit-text actions parameter caused a change in a tiddler which in turn updated the content of the modal. However, the hook seemed to fire after the “background” wiki had updated due to the change, but before the modal was updated.

No other hook was fired after the modal had been updated eventually.

Is this really so? And if yes, is it the intended behavior?

Have a nice day
Yaisog

Howto suspend background page refreshes when a modal is open?
#2

Hi @Yaisog modals are rendered with their own separate renderer. The th-page-refreshed hook is associated with the renderer used for the main page; it is not invoked for other renderers, such as the ones used for modals, or the page title, or the stylesheets.

#3

Thank you @jeremyruston for this explanation. Is there any other hook I can latch onto if I need to know when a modal has updated?
If not, could we maybe get one, or is this too exotic?

#4

Hi @Yaisog

Not that I am aware of. I guess one could write a plugin that overrode $tw.utils.Modal.

No, I think it’s reasonable, do feel free to prepare a PR.

#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.