-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce background actions and media query tracker mechanisms #8555
base: master
Are you sure you want to change the base?
Conversation
Confirmed: Jermolene has already signed the Contributor License Agreement (see contributing.md) |
return tiddlers; | ||
}; | ||
infoTiddlerFields.push.apply(infoTiddlerFields,getResultTiddlers()); | ||
mqList.addListener(function(event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO .addListener()
was deprecated. See: https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList/addListener. It should be mqList.addEventListener("change", function(event){
now
@@ -0,0 +1,5 @@ | |||
title: $:/core/wiki/config/MediaQueryTrackers/DarkLightSwitch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO the name should be DarkModePreferred
instead of switch. The state itself does not switch anything. It only tells us about the browser or os preferred setting
@@ -19,6 +19,8 @@ System tiddlers in the namespace `$:/info/` are used to expose information about | |||
|[[$:/info/browser/language]] |<<.from-version "5.1.20">> Language as reported by browser (note that some browsers report two character codes such as `en` while others report full codes such as `en-GB`) | | |||
|[[$:/info/browser/screen/width]] |Screen width in pixels | | |||
|[[$:/info/browser/screen/height]] |Screen height in pixels | | |||
|[[$:/info/browser/darkmode]] |<<.from-version "5.3.6">> Is dark mode enabled? ("yes" or "no") | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it may be Is dark mode preferred
✅ Deploy Preview for tiddlywiki-previews ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
The new filter tracker is a generic JS mechanism for tracking changes to the results of filters and changes to the tiddlers identified by those results. The rationale for the new mechanism is that it is a generalisation of code that is already present in the keyboard manager. The intention is to refactor the keyboard manager to use the filter tracker in due course
core/modules/filter-tracker.js
Outdated
filterString: filterString, | ||
fnEnter: fnEnter, | ||
fnLeave: fnLeave, | ||
fnChange: fnChange, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% sure, but there seem to be no tests if the fn*
parameters are undefined of if they actually are function()
1. Pass params as an options object 2. Add fnProcess which is called during processing after enter, leave and change have been called
Background actions are invoked when the results of a filter change. The sample background action is just for demo purposes to show how an automatic dark/light mode feature would be powered
I've updated this PR to incorporate the functionality that I had been exploring in #7999. See the updated OP for more details. |
Yes, the sample background action that is included in the PR changes the value of $:/palette, which sets the wiki dirty. As we've discussed in #7830 I don't think this approach is usable in production; the sample background action is a temporary demo. |
}; | ||
|
||
/* | ||
Add a tracker to the filter tracker. Returns null if any of the parameters are invalid, or a tracker id if the tracker was added successfully. Options include: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method currently always returns a tracker id regardless of the validity of the parameters.
if(self.hasChanged) { | ||
self.hasChanged = false; | ||
self.wiki.invokeActionString( | ||
self.actions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should provide access to global variables.
fnProcess: function(changes) { | ||
if(self.hasChanged) { | ||
self.hasChanged = false; | ||
self.wiki.invokeActionString( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we process all tracked filters before we start invoking the corresponding actions? Otherwise the actions invoked by the first filter can trigger subsequent filters in the same cycle, as opposed to being deferred to the next refresh cycle.
This PR introduces two new general purpose mechanisms for end users:
Both these mechanisms are based on a new "filter tracker" JavaScript class that is also intended to have other applications (eg tracking keyboard shortcuts).
The use of the info mechanism for the CSS media query tracker means that these tiddlers are dynamically created as shadow tiddlers within the
$:/temp/info
plugin, and so do not appear in tiddler lists.The mechanism is used to implement the existing dark mode tiddler with the following configuration:
Note the use of
info-tiddler-alt
to specify a redundant secondary info tiddler. This is used by the dark mode tracker to maintain compatibility while changing the info tiddler title for consistency.This PR also includes a rudimentary background action that is invoked when the operating system dark mode setting is changed. It is only included for demonstration purposes, and shows how the media query tracker can be used in association with background actions.
This PR was prompted by @pmario's work on dark mode switching in #7830 and is intended to allow us to implement actions for dark/light mode changes in a more extensible and generic way.