FactorFence Visualizer

[katestange]

By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.

---

This is a factor visualizer. It takes the terms of the sequence s(n) and creates a bar of height log(s(n)) at position n. The bar is divided into pieces of height log(p) for each prime p in the prime factorization (with multiplicity).

Some features:

1. There's one "highlighted" prime, which is always shown at the bottom of the bars. If you click off the graph, highlight is removed. If you click on a prime, it switches that prime to the highlight role.
2. If you mouse-over the graph, information on the bar and prime you are hovering above is shown below the graph
3. You can use the mouse scrollwheel to zoom, and the arrow keys to move around; there are a few other special keys
4. It treats signs separately (checkbox turns this on/off): the sign is removed before taking log and slapped back on afterward.

A few interesting ones:

http://localhost:5173/?name=5350&viz=FactorFence&terms=1000&highlight=0&seq=OEIS+A005350

http://localhost:5173/?name=abc+hits&viz=FactorFence&terms=1000&highlight=0&seq=OEIS+A130510

[gwhitney]

OK, I modified the types so that the lint/typescript special cases wouldn't be needed. But I am observing a significant issue in that now when I scroll the parameter tab for the FactorFence visualizer, the visualization also zooms. It should only zoom when the mouse cursor is over the canvas. Are you observing that same issue? If so, I think it needs to be fixed up before this is further reviewed. If you are having trouble with that, let me know and I will be happy to help.

[katestange]

OK, I modified the types so that the lint/typescript special cases wouldn't be needed. But I am observing a significant issue in that now when I scroll the parameter tab for the FactorFence visualizer, the visualization also zooms. It should only zoom when the mouse cursor is over the canvas. Are you observing that same issue? If so, I think it needs to be fixed up before this is further reviewed. If you are having trouble with that, let me know and I will be happy to help.

Ok, I agree with this and noticed it also. I think probably this is a "global" issue, not something each visualizer should have to handle. That is, mouse events should be captured and then routed to the canvas only if the mouse is over the canvas (and not over a modal), and the default browser behaviour prevented. So I think this is probably a modification to frontscope, not FactorFence. Should it still go in this PR? Or a separate one? I found a few relevant things, e.g. here. It looks like the idea is to add an event listener to the canvas when it is created.

[gwhitney]

Ok, I agree with this and noticed it also. I think probably this is a "global" issue, not something each visualizer should have to handle. That is, mouse events should be captured and then routed to the canvas only if the mouse is over the canvas (and not over a modal),

I think the thing is that the parameter tabs are children of the canvas in the DOM tree, so mousewheel and other events are naturally bubbling up from them to the canvas. At least, I think that's it. So I think that all that is needed is to specify in Vue that the parameter tabs stop the propagation of wheel and click events. I think there's a standard way to do that in Vue. Again, if you are having trouble finding out how to do that, just let me know.

I agree that conceptually this was a pre-existing bug in ui2, not really specifically to do with FactorFence. But as FactorFence is the first visualizer to exercise the bug, it seems convenient, expedient, and sufficiently appropriate to just fix the bug along with this PR that adds FactorFence.

[katestange]

I think the thing is that the parameter tabs are children of the canvas in the DOM tree, so mousewheel and other events are naturally bubbling up from them to the canvas. At least, I think that's it. So I think that all that is needed is to specify in Vue that the parameter tabs stop the propagation of wheel and click events. I think there's a standard way to do that in Vue. Again, if you are having trouble finding out how to do that, just let me know.

I tried this, but no luck so far. But we have the same issue on the navbar, which I don't think is a child of the canvas?

[gwhitney]

I tried this, but no luck so far. But we have the same issue on the navbar, which I don't think is a child of the canvas?

Weird. Maybe I am misuderstanding how this works. I will pull and take a look.

[gwhitney]

I will pull and take a look.

Aha. I just found the following in the p5 source code:

// Bind events to window (not using container div bc key events don't work)

    for (const e in this._events) {
      const f = this[`_on${e}`];
      if (f) {
        const m = f.bind(this);
        window.addEventListener(e, m, { passive: false });
        this._events[e] = m;
      }
    }

So there you have it. p5 is explicitly grabbing all events that happen anywhere on the entire window, not just ones that happen on its own element(s). That's pretty poor citizenship, if you ask me, and I would never have thought p5 would do that -- that's why I assumed we were just not propagating events correctly. We will need to work around this somehow, and I agree, the workaround should be in p5Visualizer.ts rather than in individual visualizers. I will think on this -- too tired to try to work it out now, I just finished a big construction event here at PCMI/IAS.

[katestange]

So there you have it. p5 is explicitly grabbing all events that happen anywhere on the entire window, not just ones that happen on its own element(s). That's pretty poor citizenship, if you ask me, and I would never have thought p5 would do that -- that's why I assumed we were just not propagating events correctly. We will need to work around this somehow, and I agree, the workaround should be in p5Visualizer.ts rather than in individual visualizers. I will think on this -- too tired to try to work it out now, I just finished a big construction event here at PCMI/IAS.

Maybe the poor citizenship is so you can do things like this. That shows how one can pay attention to mouse position from within p5.

[gwhitney]

Maybe the poor citizenship is so you can do things like this. That shows how one can pay attention to mouse position from within p5.

That example uses mouseenter and mouseleave events, with listeners manually added to the canvas element, rather than automatically added (to the entire window) by the p5 library itself. In fact, mouseenter and mouseleave are among the few events that p5 doesn't touch.

So I had some time to contemplate how to proceed here. I in fact considered the possibility of our adding our own listeners to the canvas. But (A) there is some p5 boilerplate wrapped around the sketch's mouseWheel() and other functions, and to make our environment as much like "ordinary" p5 as well, we probably want that boilerplate and we don't want to copy its code, because that might change out from under us in a future release of p5. And (B) p5 is still putting numerous event listeners on the top level window no matter what we do -- even for visualizers that define no event functions, those listeners are there and they fire on every wheel event, etc., just to repeatedly check that nope, this sketch still doesn't have a mouseWheel() function. (Another example of unimpressive coding in p5 -- why check on every event? Seems like it should check at the beginning when the sketch is created, and not bother putting a listener if nothing is going to happen. The documentation doesn't say anywhere that you can add a mouseWheel() function to a sketch after it is created, and adding methods to objects after the fact is a very unusual thing to do, so I don't think anybody would miss it if p5 lost the ability to start calling a mouseWheel() function that was added after sketch creation.)

So anyhow, I think it would be confusing if there were the p5 event listeners plus our own, and the p5 ones are going to be there anyway, nothing we can do about it, so we may as well use them. I then wondered if the base class implementations (in the WithP5 class) could check if the event occurred in the canvas and if not, drop them. But that would mean that the individual visualizers would be obliged to call super.mouseWheel() -- not so bad -- and then somehow check the return code to see whether or not to process the event; but that same super function with its return code would have to work on its own as an event handler in the more common case when the individual visualizer doesn't define an event handler. So then that got too confusing for me also.

So my intention at this point is instead in our sketch creation function (inside the inhabit() method of the base P5Visualizer class factory), where for example it generates the draw function with special code to change the cursor in case of a cache miss, to similarly handle certain of the mouse event functions specially and ignore them (i.e., never call the visualizer functions) if they occur outside the canvas. I think the ones that need special handling are mousePressed(), mouseClicked(), and mouseWheel(). I think we want to ignore all of those happening outside the canvas. I don't think we should do anything to mouseReleased(), mouseMoved(), or mouseDragged(), because if a visualizer gets a mousePressed it might legitimately want to know that the mouse is released outside the canvas; and it might want to track motions or drags into or out of the canvas. Any other events that perhaps need additional special handling? I guess there are key events -- I will have to check to see if they are "bleeding" into the visualizer as well, and if so, maybe check whether the canvas has focus and if not, ignore them.

Not sure when I will get to this, though, sorry.

[katestange]

Ok, so this is ready to be reviewed or to get some help with, I'm not sure I can do more here. I tried to work toward having the parameters dialog complain when the user-entered number of terms exceeds the number available, but I ran into two problems: (1) I can't access seq.last and seq.first from inside the params object, so I can't display to the user the number of terms or check at that moment; and (2) if I add a validation in checkParams() then it produces all sorts of weird behaviour, probably related to the issues mentioned above about refreshParams(), so I kind of gave up...

[gwhitney]

OK, I should have time later this week to look at/take care of the three outstanding items on this:

• [x] Fix up P5Visualizer.ts per the above comment so that click and wheel handlers don't fire on events outside the canvas;
• [x] Fix whatever weird thing is going on with refreshParams() in checkParameters() (see below)
• [x] Get the allowed ranges into the description strings for parameters like the number of terms. (This may involve enhancing the framework so that those description strings can be updated dynamically, rather than just their initial value be used. However, that bit may work as things stand, I am not sure.

If anyone else sees/remembers any other aspects of this PR that need attention before this is merged, please post them here. Thanks.

[gwhitney]

All right, I pushed a commit that prevents mouse presses, clicks, and wheel events that occur outside the canvas from being handled by the Visualizer. Please test and see if it works for you. However, I have not checked the item off in the checklist above because there is another wrinkle. Suppose you are running FactorFence, and you are currently viewing 1000 terms and you want to change that to 2000. So you click in the text entry box for the number of terms, and say you are not clicking so precisely, and the cursor ends up after the last "0" in "2000". You might naturally hit the left-arrow three times, but if you do, the cursor will move to just after the "2" but the visualization will also scroll three bumps to the left. That behavior is a bit jarring to me. But how about if your last click happened to be in a blank area in the top bar that has the numberscope link and the name of the visualization etc. Should the visualization at this point respond to the arrow keys? If so, then it's not as simple as just checking whether the canvas has focus to decide whether a keypress should be paid attention to by the visualizer. Or maybe we don't mind the "double response" to arrow keys when used in the parameter tabs or to the "y" key if the person wants to name the visualization some name that includes a "y"? Anyhow, I need guidance as to how/when/where the keypresses should be responded to in order to finish this part off. Thanks for your thoughts.

[katestange]

About keypresses. Hmm, that's tricky. I hadn't noticed it when I was messing around, so it's a fairly uncommon situation. Three options I can think of: (1) leave it alone; double behaviour is ok (jarring as you say) (2) pay attention to keypresses only when mouse is on canvas (probably sometimes frustrating, i.e. an overcorrection) (3) pay attention to keypresses only when canvas has focus (also probably sometimes frustrating/overcorrection) Given the lack of other options, my current favourite is (1). I'm worried that (2) and (3) or any combination thereof will result in confused users who are wondering why keypresses don't work. There isn't any way to know if you are currently moving a cursor, is there, like "focus is in an entry field"?

[gwhitney]

(1) leave it alone; double behaviour is ok (jarring as you say)

I'm fairly against this because it feels like "we coded it wrong".

(2) pay attention to keypresses only when mouse is on canvas (probably sometimes frustrating, i.e. an overcorrection) (3) pay attention to keypresses only when canvas has focus (also probably sometimes frustrating/overcorrection) Given the lack of other options, my current favourite is (1). I'm worried that (2) and (3) or any combination thereof will result in confused users who are wondering why keypresses don't work.

On the other hand, I don't disagree with you there, either. This seems like a difficult dilemma (and is presumably why p5 defaults to just handling all keypresses).

There isn't any way to know if you are currently moving a cursor, is there, like "focus is in an entry field"?

I am not quite sure what you mean by "moving a cursor"? The issue occurs for just typing a "y" or an "h" when you are naming your visualizer, as well.

In our case, since we fully control the layout, we do in some sense have a list of all of entry fields, so in principle we can check each one to see if it has focus. And maybe that's what we have to do. But because these entry fields are in different components, that approach will keep the code from being as modular as it is now -- the visualizer will have to know about the name entry bar and the parameter tabs and so on, which right now it doesn't care about. And when we add something like the OEIS search bar, the list needs to be updated... We should keep brainstorming about how to handle these keypresses.

[gwhitney]

We should keep brainstorming about how to handle these keypresses.

There seems to be something called document.activeElement that could be helpful. One of us should put a console.log of that in the keypressed handler and see what it says in various situations in which we would, or would not, want the visualizer to be handling the keypress, and see if we can determine a criterion that could be used at the P5Visualizer level to decide whether a given keypress event should be passed to the visualizer to handle. If nobody else gets to this before I have a chance, I will do it.

[gwhitney]

A comment to https://stackoverflow.com/a/30714894/5583443 suggests that

if (document.activeElement.tagName === "INPUT")
    console.log("An input is focused");

is our ticket, and noodling around, it does seem promising. When I get a chance, I will add a commit making P5Visualizers ignore keypressed events when an input is focused by the above heuristic, and we can see if that gives us intuitive behavior.

[gwhitney]

OK, the keypresses seem OK to me now, so checking off the event handling above. Let me know if you agree. As I was working on this, I noted that if you enter '6' as your favourite prime, it accepts it, dutifully shows "highlighting prime: 6" under the bar graph, and then of course does not highlight anything. Feels a little inconsistent? Should it complain that 6 isn't prime, or highlight both 2 and 3, or something else? I will leave this to your decision, @katestange; please just let me know if you will be adding a commit to make a change to this behavior or if we will just forge ahead as is. Thanks.

[gwhitney]

P.S. One could change the field to "highlight factors of" (call the value h) and then use the quick gcd algorithm h mod p === 0 to deal with an arbitrary integer input without ever having to factor.

[katestange]

P.S. One could change the field to "highlight factors of" (call the value h) and then use the ~quick gcd algorithm~ h mod p === 0 to deal with an arbitrary integer input without ever having to factor.

Beautiful idea -- it extends functionality and at basically no cost. I'll do this.

[katestange]

Ok, pushed a few changes:

1. favourite number has to be positive, it will check for all factors
2. refactored the display of factorization under mouse
3. the display of factorization now shows three colours matching the bar: highlight, mouseover and plain
4. Change Y/H to I/K for stretching

[gwhitney]

OK was looking into my second checkbox, about refreshparams in checkparameters, but then I b remembered that we had an in-depth conversation about how we want to steer Numberscope away from forcibly changing your direct input to accepting any input that meets the formal validity requirements and then warning the user that it's actually going to behave as though the different and more acceptable/usable number had been entered.

So I am reluctant to merge another example of the behavior -- altering the user's keystrokes -- that we are deprecating and will have to change anyway. So it seems to me that we will never need refreshParams to work from checkparameters, so there's not much benefit to spending time on that now.

But that leaves the question of how to change factorfence for merging now. First, clearly it should not change the terms param, but instead just behave as if the smaller number of terms had been entered. But it should then also provide feedback as to what it is doing. But there is not yet any "warn" status. So should we implement the "warn" status in this PR, or just not say anything about the actual terms being lower than the requested terms until a later PR that implements such warnings?

[katestange]

But that leaves the question of how to change factorfence for merging now. First, clearly it should not change the terms param, but instead just behave as if the smaller number of terms had been entered. But it should then also provide feedback as to what it is doing. But there is not yet any "warn" status. So should we implement the "warn" status in this PR, or just not say anything about the actual terms being lower than the requested terms until a later PR that implements such warnings?

Yes, exactly my concern also. I am ok with just using fewer terms and then filing an issue to create the "warn" status very soon, but if you'd like warn implemented in this PR, I'm ok with that too (and could even try doing it). Just let me know.

[gwhitney]

All right well I don't mind using this as a vehicle to get that done. So if you want to take a stab, great. You will need to add a new ValidationStatus for warnings that leaves the whole thing "ok()" but which also displays the messages (say in orange rather than red, or something like that).

[katestange]

All right well I don't mind using this as a vehicle to get that done. So if you want to take a stab, great. You will need to add a new ValidationStatus for warnings that leaves the whole thing "ok()" but which also displays the messages (say in orange rather than red, or something like that).

Ok, I was being too ambitious. I don't think I can figure out quite what's needed to do this! Can you take a stab?

[gwhitney]

OK, I have indeed taken a stab at it. Let me know what you think. I rebased on the new ui2 so that I would be able to search the OEIS while testing it. I encountered the following concerns that I think should be fixed before we merge this:

1. There seems to be an off-by-one error (I may have introduced it...): When I ask for three terms, I get two bars; for four terms, three bars; etc.
2. This situation (of point 1) is made slightly more confusing in that I believe that sequence values of -1, 0, 1 don't appear at all. Should you put dots or dashes at the baseline as placeholders for those terms or something like that?
3. With the switch to "favourite number" it seems that clicking on the canvas but not on a bar sets that favourite number to 0, highlighting everything (rather than nothing). I think it should set the favourite number to 1 (especially because on reload, the 0 is flagged as an invalid value for the "favourite number").

When you've made and pushed whatever responses you feel are appropriate to these points, I will proceed with getting the available number of terms injected into the description string for the terms parameter.

[gwhitney]

Point 2 above is especially acute for sequences like A010060 Thue-Morse that only have entries in {-1, 0 , 1} -- the FactorFence visualizer produces a completely blank screen except for the legends, which feels like it's not running at all...

[gwhitney]

Ah, and point 3 above has revealed that the concern about which clicks get to a visualizer has not yet been fully resolved: if you have the FactorFence visualizer active and you open the sequence switcher and click in the OEIS Search entry area, the "favourite number" goes to 0, which does not seem appropriate. The point is that click is making it to the Visualizer because it is geometrically within the bounds of the visualizer canvas, even though it is inside a popup that is currently being displayed on top of that canvas. I will have to think on how to prevent this kind of click getting to the Visualizer, and can try to address it after @katestange has made whatever response she decides is appropriate to points 1,2,3 that I recently raised.

[katestange]

Ok, I've pushed commits addressing 1-3. I fixed the off-by-one error, I made outside clicks make favourite = 1, and I added a thin dash line to show a "placeholder" for terms that are 0,1,-1.

[katestange]

Oh I see a tiny thing that needs fixing also, hang on.

[katestange]

Ok, I fixed a bug in the colours of factors in the displayed factorization info. @gwhitney you are welcome to do your next thing

[gwhitney]

All right, I think I have fixed the issue that clicks on the popups (such as in the OEIS search bar) were bleeding into the visualizers. I also had to make some changes in terms of waiting in the Scope for sequences to fill their cache before trying to validate the visualizer, because a sequence doesn't know how many terms it has until its first cache fill, but that info is used in the checkParameters of some visualizers (including this one).

Try it out and see what you think. When I have a chance, maybe tonight, I will look into updating the description strings with the max number of terms.

[gwhitney]

Oh, I also noted that when you use the k key to move the factor fence down, the caption with information about the keys you can use and the sequence value and its factorization does not move with it. So the bars start overwriting the captions. Is that intentional, or do you want to push a commit that moves the captions as well (or some other resolution to this point)? Thanks, Glen

[gwhitney]

Actually, I had a few minutes to look at updating the prompt for the number of terms to show the maximum, and it turned out to be entirely straightforward. So I've pushed a commit that takes care of that. You can pull and try everything, and decide what you want to do about the captions moving or not when you move the bars up/down. Once you've let me know you're done with that (and have pushed anything you might want to, or decided it is OK as is on that score), I will do a final check/review and merge this visualizer -- there's nothing else on my radar for this PR.

[katestange]

(1) I have a suggestion about the focus on textboxes: enter should cause the textbox to lose focus. I find myself changing a setting, then the arrow keys cease to work and I don't realize why and the only way to get back to having arrow keys working is to click on the canvas (which has its own effects, namely changing the favourite number, so not ideal), or to click somewhere outside an entry field on the parameters pane, which is a very specific action that I had to discover by trial and error. But hitting enter to finish entering something in a text box is a very natural action.

(2) When I click a checkbox I also lose the ability to use arrow keys. That's not a desired behaviour; I think checkboxes shouldn't be included in the "entry field focus stealing key activities" rule.

[katestange]

May I suggest a wording change to the wording of the warning? Currently it is: "Displaying all 16091 terms of sequence, fewer than the 1770000 requested." How about "More terms requested than available; using only 16091." It's shorter/punchier?

[katestange]

Oh, I also noted that when you use the k key to move the factor fence down, the caption with information about the keys you can use and the sequence value and its factorization does not move with it. So the bars start overwriting the captions. Is that intentional, or do you want to push a commit that moves the captions as well (or some other resolution to this point)? Thanks, Glen

Ok, I played around with it and I saw a few options. Keep in mind the text can always be turned off:

(1) leave as is; it can overlap the graph (2) have it scale with the graph; then it can end up partially off the screen (3) have it disappear when the graph starts to overlap it; but disappearing might be annoying if you still want the text (4) put a background box on the text so it obscures the graph when there's overlap; the size of the text changes with mouseover, so the box would always be popping around in size, which is unpleasant (5) fill the entire bottom of the canvas with a background upon which to put the text, which the graph will slide "behind" when it moves around; this reduces screen real estate a fair bit. It could be pushed low to make a sort of "footer" of text, but then your eye has to move further as you mouse over and play. (6) use a font that is appropriate for movie captions (i.e. has some background or contrasting border); I think it looks worse (7) have the text in a pop-up mouseover, like in histogram; I find this less visually appealing and too jumpy

I didn't have any other ideas, and decided that I personally prefer (1) ... and (5) is my second choice. Any input or other ideas?

[katestange]

All right, I think I have fixed the issue that clicks on the popups (such as in the OEIS search bar) were bleeding into the visualizers. I also had to make some changes in terms of waiting in the Scope for sequences to fill their cache before trying to validate the visualizer, because a sequence doesn't know how many terms it has until its first cache fill, but that info is used in the checkParameters of some visualizers (including this one).

Try it out and see what you think. When I have a chance, maybe tonight, I will look into updating the description strings with the max number of terms.

This appears to work and block clicks. Mouseover is still picked up by the canvas but in the case of this visualizer, I see no problem with that. Perhaps some other visualizers would dislike the behaviour.

[katestange]

The example image needed to be updated (it is now out of date with various changes). Committed.

[gwhitney]

I didn't have any other ideas, and decided that I personally prefer (1) ... and (5) is my second choice. Any input or other ideas?

I guess I don't understand why the bars slide up and down with i/k but the text stays put; why not just slide the text up and down by exactly the same amount as the bars are sliding up and down?

[katestange]

Ok, now it does. It can still overlap the graph when using U/O however. What are your thoughts on that?

[gwhitney]

It can still overlap the graph when using U/O however. What are your thoughts on that?

Well, it only happens when you turn on respecting signs, and then stretch the bars. So it feels less like a bug to me. I'm OK living with it.

But if it gives you discomfort, you could just calculate the minimum level any bar goes down to and make sure the legend is below that, unless that would put it off screen; otherwise it's just as low as it can be while still on screen (and then yes, it does overlap the bars, but at that point it's "forced" to -- there's no screen space left where it wouldn't overlap).

Since it seems like there will always be some chance of overlap, you might want to change the first line of the legend to use the green color from the palette rather than the purple. Just a thought.

I'll leave this point in your hands to wrap up or just to say that you're also OK with how it is now and you're leaving it be.

[gwhitney]

OK, reworded the warning as suggested; text fields lose focus on enter; and boolean fields lose focus after every interaction.

However, I did encounter another concern: I got the "Tau Many Primes" visualization in a nice state with the bars the way I wanted and zoomed the right amount and then wanted to highlight the '2's to check if in fact this sequence is only odd when the index is the square of an odd number. But typing a 2 in the "favourite number" box reset the scaling and positioning. I think that's because by default updating a parameter always calls setup, and setup initializes all of those aspects. So that was frustrating. It seems to me that changing the highlight number should leave all the other aspects the same.

I see at least four ways to go here:

A) Leave the behavior as is --- maybe it's good to start with a clean slate whenever any parameter changes, even if sometimes people are a little frustrated with having to "get back to where they were".

B) Make the zoom, vertical scale, left/right position, and up/down position params as well. They would start with their default values, and the keystrokes would change them and use refreshParams() to show the new values, but you could type directly into the params instead/as well if you like.

C) Figure out where to initialize these internal values (maybe the constructor?) and then don't touch them in setup so that they will keep their state even when the favourite number (or number of terms, for that matter) is changed.

D) Override the parameter update function so that it can leave these internal values alone when the favourite number or number of terms is changed.

I'm leaning toward B because it will have the side effect of putting that info in the URL, making the exact view that someone has achieved reproducible when they share their visualization. But you may object to (B) on the grounds that t will complicate the ui in the visualizer parameters tab. We could minimize that complication by hiding all these view parameters under a checkbox (like with the "additional dot parameters" for the Chaos visualizer).

So anyhow, I will leave which way to go with this up to you. Just let me know how you'd like to proceed and if there are to be any changes, whether you would like to execute them or prefer that I do.

[gwhitney]

P.S. A consideration about (B) from my last post is that it would cause all of those view aspects -- stretch, position, zoom -- to be kept when changing the sequence as well. You might prefer that whenever the sequence changes, the view resets to the "standard view". If so, that would argue for something like (C) or (D). I think (C) would work with the view variables initialized in presketch() -- I think that presketch() is called whenever the sequence changes but that it is not called when a param changes (someone would need to check on that to be sure).

[katestange]

Regarding the pan/zoom parameters. At first, I was leaning toward (B), but then it occurred to me that even if they are in the URL and shareable, the person who loads them next time may have a different canvas size or aspect ratio, so the effect might really be different anyway. This is true of any visualizer, but it's particularly pertinent to these "positioning" parameters. Then I thought about your comment on (C) and whether these are reset with a new sequence -- I definitely want to go back to 'standard' view with a new sequence. And for that matter, loading the standard view is reasonable when we have a canvas resize or someone else loads the visualizer in a different aspect ratio. The visualizer goes to some lengths to compute a good initial positioning based on the sizes of the terms and canvas, and giving that up would ruin a good thing and cause a lot of annoyance and messing about for the user.

So I guess I'd go with (C). I also think this is a reasonable choice in the wider context of varied visualizers, because I think there may be other cases where we want to differentiate code that runs when a new sequence is loaded vs. when a parameter changes.

So to sum up, a possible proposal would be:

1. Reset to standard view on resize, share URL, or new sequence, but not on other parameter changes
2. Capability for visualizers to have code that runs on new sequences but not on parameter changes (possibly presketch() )
3. Capability for visualizers to have code that runs on canvas resize but not on parameter changes (does this exist?)

What do you think?

Related question: Does the current design allow for a visualizer to trigger certain code when parameter A changes but not when parameter B changes?

[katestange]

I updated the text color as suggested.

[katestange]

OK, reworded the warning as suggested; text fields lose focus on enter; and boolean fields lose focus after every interaction.

This all seems to work as advertised.

[gwhitney]

So to sum up, a possible proposal would be:

1. Reset to standard view on resize, share URL, or new sequence, but not on other parameter changes

2. Capability for visualizers to have code that runs on **new sequences** but not on parameter changes (possibly presketch() )

3. Capability for visualizers to have code that runs on **canvas resize** but not on parameter changes (does this exist?)

What do you think?

1. All the points you make are reasonable, totally happy to go with this plan. I do not think presketch() will run on resize, but I think there is an explicit resize method that one can extend. So I think we can make a method "resetScaling" and call it in on presketch() and resize(). I am pretty sure that presketch() is called on both new sequence and new URL (and opening gallery/saved visualizer), so then I think that will fit the bill.
2. Yes, I think presketch() fits this.
3. Yes, I think there is a resize() method.

Related question: Does the current design allow for a visualizer to trigger certain code when parameter A changes but not when parameter B changes?

Yes, there is a parameter update function, which currently just always calls reset(). But if you extend or override it, you can see what parameter has changed and take action accordingly. In addition, I am thinking we should have a "leave" option in parameter descriptions which, if specified to be true, means that adjusting that parameter (like the checkbox to reveal other parameters) does not reset the visualization. (But there is no such option at the moment, you basically need to roll your own using the parameter update function at the moment.)

So I think this can move ahead. I will assume that you would like me to do the implementation unless I hear that you want to take a stab at it.

[gwhitney]

Whoops, nope, a quick experiment showed that unfortunately presketch is called on any arbitrary parameter change. Ugh. So I will look into the framework and try to find an apt place to insert a method that is called on new sequences but not called on an arbitrary parameter change, or else maybe I will change things so that presketch is not called on parameter changes, since to me it doesn't seem logical/reasonable that it should be called just for a parameter change. I am leaning toward that solution -- make sure presketch not called on parameter changes -- unless I hear feedback from anyone otherwise. But I will take care of the implementation.

[gwhitney]

OK, I have adjusted the framework so that presketch(), as we had originally expected, is not called on resize or on parameter change (but it is still called for a new URL or a new sequence). It was a pretty small easy change, so if you don't think that's the right direction, it would be easy to back out. With that, let me know if you want to implement the planned changes to FactorFence in terms of when to standardize the view: on new sequence/URL and on resize, but not on other parameter changes. (Since presketch() is not called on resize, there will need to be a "standardizeView()" or some such method that is called both from presketch() and in the resized() function. )

Some implementation notes:

• In presketch, this.sketch is not available, so in this plan in "standardizeView()", you can only do things that do not involve accessing this.sketch. Anything that does need sketch access has to be left in setup().
• In presketch, remember to await super.presketch()
• In your resized() function, you will presumably want to call "reset" after standardizing the view, to redisplay the visualization. By defining resized(), you will have prevented the default behavior on resize of just calling reset(). So you need to call reset yourself if you want it called.

I am fine either way with your making the changes about standardizing the view, or doing it myself if you prefer. Just let me know.

[gwhitney]

P.S. Give me a sec to update the docs in making-a-visualizer.md about when presketch is called. Once you see a commit with the message "doc: Detail when presketch() is called" you can have at it, if you are planning to do the implementation.

[katestange]

Ok, I can think of visualizers that would want to rerun preSketch() on change of parameters if that parameter affects the sequence precomps that take a while, e.g. suppose I added a "translate" parameter to the factor visualizer, so you could translate the sequence values by 1 (so t_n now becomes 1+t_n) and then factor -- all the factoring would need to be redone. You might say, ok, that should really happen on the sequence side, we shouldn't have the visualizer controlling translation. And you'd probably be right about that, certainly in this example. Although perhaps there are situations where there's a more visualizer-based parameter change with a big precomp... Is this a situation we want to worry about at all?