-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: Save a history of any kind of Sequences (not just OEIS) #500
Conversation
In meeting, decided to just have a toggle between "thumbnail view" and "list view" for all items, and to always show them all. |
Also noted that we need to add a sequence when a switcher is opened, and clicking on any card should save the destination of that card. |
OK, the tracking and most-recently-used designation of sequences in the history should now match what we discussed at meeting. Just need to implement the toggle switch and the list view for this to be ready to review (although feel free to pre-review more as you like and let me know if you see anything). |
This partially resolves numberscope#430; the only portion remaining is to have a limit on the number of sequences that will show as thumbnails (the rest will show as a table or something like that). Also, testing for this commit revealed that the p5.brush addon does not work properly with multiple different sketches all updating asynchronously. Therefore, this commit removes p5.brush and provides a simple hatchRect() method in P5Visualizer (since hatching is all that p5.brush was being used for anyway). The current sequence is saved/updated in the list of remembered sequences when (a) the Scope view is first loaded; (b) the Scope view is closed; (c) a sequence or visualizer is selected in the SwitcherModal; or (d) the current specimen is saved to the Gallery. This commit removes the former list of OEIS IDs that was being saved in browser localStorage, as it is superseded by this sequence-saving feature. That removal means that the frontscope no longer (pre-)fetches a collection of OEIS sequences immediately on startup; they are loaded only as used.
Also save a sequence when the switcher is opened (so there is no need to save it when a new sequence is selected, since the switcher must have been opened in order for something else to be selected), and save the destination sequence any time a specimen card is selected. Also puts the mod information in the description of an OEIS sequence so that we don't end up with a bunch of identical-looking sequence cards.
ef4fd13
to
361a65b
Compare
Also adds a simple toggle switch component for selecting how a given SpecimensGallery should be displayed. Makes the title and subtitle properties for a specimen card mandatory (as they are displayed in the table view). Adds the visualizer as well as the sequence to the subtitle for featured and saved specimens so that the table view would be more informative (and the extra information also shows up in the thumbnails view. This commit significantly reorganizes the HTML/CSS of a SpecimensGallery component to accomodate the toggling between the two views (not to mention the non-scrolling control to do that toggling). All of the same abilities (select a specimen, delete a specimen, link to OEIS) are present in the table view as in the thumbnails view. Canonicalizes the query string of a couple of the featured items, to prevent them from apparently creating duplicate saved sequences.
All right, there are now working list and thumbnails views in any of the SpecimensGallery choosers that appear (featured specimens in the Gallery, saved specimens, sequence switcher, and visualizer switcher). There is a toggle switch to choose between the views. The thumbnails view is the default. This now implements everything mentioned in #430, so marking this as ready for review. I wasn't quite sure about the layout/graphic design of the toggle control, so I am definitely open to feedback/suggestions about how to present that. |
One thing I notice is that if I add a ton of OEIS sequences, the "standard" (non-deletable) thumbnails get pushed off screen (e.g. Formula). Is this desired? |
When the formula gets long, it can push the chooser button offscreen to the right. Here's an example in firefox: |
I like the list view and the toggle! My only initial suggestion there would be to use the word "list" instead of "table". Also, and this may not be something to deal with in this PR, but just to file an issue for (?) but when I've put enough items in my switcher to make the list have a scrollbar, toggling back to "thumbnail" is laggy, presumably from refreshing so many thumbnails. |
Should the sequence switcher remember the state (either list or thumbnails)? Right now if I'm using list and I go to a specimen and then back to the switcher, it reverts to thumbnail view. |
Well, that's a consequence of the current "most-recently-used" sort order. Am totally open to other suggestions. We could always "float" the most recently used OEIS, most recently used Formula, and most recently used Random to the head of the list, for example. I am going to leave this as-is unless/until there's a consensus on a specific idea to try. |
OK, I will look into "prettyprinting" the formula into the title of the cards so that it can wrap more gracefully. (Note this issue is not new to this PR, same thing happens in ui2 as it stands, but fine with fixing it here.) |
Presumably that will be fixed by not rendering off-screen thumbnails, which is slated for after alpha. So yes, this could go in a comment on that issue. |
Should it? I made "thumbnails" always the default because it's so pretty. But if there's a consensus that remembering is better, that's possible -- we would just have to decide whether it remembers just within one Numberscope "session" or across runs (i.e. would need to use browser local storage). Another option is to default to "thumbnails" under some fixed number of items (say mid-teens) and default to list when longer than that, since list is much more compact. (Again, not taking any action here until there's a definite direction to go.) |
OK, there is not a text pretty printer, but there is a facility to convert to MathML so that the formulas would appear in tex-style typeset format (only in the titles, not in the formula entry box), i.e. the |
The thumbnail toggle works well for me overall! That said, here are two details I found confusing. Naming of list viewI'd call the no-thumbnails option a list view rather than a table view. This matches the naming convention used by file browsers in macOS and Gnome. It does conflict a bit with the Windows file browser, which calls the analogous view details, and uses list for a more compact view. However, I've never seen a file browser that uses table. Active option highlightingI initially read the blue-highlighted label for the active option as a link. I was confused about why it would be a link, and why it didn't do anything when I clicked it. For me, it might be clearer to label the active option in black and the inactive option in gray. This would remove the contrast between the options and the "Display as" text, but there are ways to either restore contrast or make the contrast unnecessary. Here are some examples; the subscripted quotes show which text the color applies to.
The last idea uses punctuation, rather than color, to separate the options from the "Display as" text. |
Right now, it seems like thumbnail loading could make the Change sequence dialog unusably slow for users with very long sequence histories, so those users need a way to make sure that thumbnails stay turned off. Since the sequence history is remembered across sessions, the thumbnail toggle would also need to be stored across sessions to make the system work well for those users. I'm not sure this is worth delaying the PR for, though. |
I'd like MathML in formula sequence titles, because it might help clear up a confusion I encountered. When I opened Numberscope for this test run, my initial sequence was Formula: (sqrt(2)n) % 3. I wanted to change the formula, so I clicked the text (sqrt(2)n) % 3, and I was confused when this opened the sequence chooser rather than allowing me to edit the formula. I actually did this twice before realizing I needed to click in the Formula input below the sequence title. By the way, this makes me more convinced that we should do more to distinguish the sequence and visualizer titles from text inputs. (If y'all agree, I can open an issue for this.) The underline really makes me think "type in this field to change it", not just "this value can be changed". In some cases, the mismatched expectations aren't so bad, because the Choose sequence or Choose visualizer dialog is the way to do what I want to do. If I want to edit a formula, though, the dialog is not the way to do it. |
I think Aaron and I both like the idea of remembering the state (thumbnail or list), but I would be ok with this being stored only across runs, as a compromise between reminding the user how pretty thumbnails are, and annoying the user who wants it to stay in list mode while they muck around extensively. |
I agree, I like MathML in formula sequence titles. It will look more professional. Is it a lot of work, though? |
After mulling this over, my current leaning is to keep the "undeletable" Formula and Random always top left, and just rotate everything else below that. This seems to match my "user expectation" from other apps and UIs best (as Aaron is often considering), and the newest/current sequence isn't exactly lost, it will always be first row. Those two are very useful and they are top of the list by default for a reason. OEIS sequences are accessible always via the search box, so I don't think they need to be reordered to keep one near the top. |
I agree that keeping the "sequence factory" items at top left would be reasonable, since they provide functionality that can't be consistently accessed any other way (as far as I know). |
Personally, I find it infuriating when an app reminds me about a feature that I've explicitly turned off. It's especially bad when the "reminder" takes the form of turning the feature back on without my consent. If a Numberscope session always starts with thumbnails turned on, the new-session workflow for someone with a long history and a slow computer might look like this:
I'd be more comfortable if we could be confident that the thumbnails will never block the interface, but I'm not sure how we'd test this. |
Yes, it turns out that chromium-based browsers have different default css for Thoughts? How hard should we work to overcome Safari's apparently slightly broken implementation of MathML? It is the second most popular browser, with a 20-25% market share. On the other hand, super-long formulas won't necessarily come up that often. On the third hand, when they do, we don't want to look like our software is broken (even if it really is Safari's "fault"). |
I would say it's ok to make a separate issue and discuss priority, unless you have a simple enough idea about how to fix. |
@gwhitney Maybe we should also title the permanent Random card "Random" (leaving off "0 to 9"). |
OK, I filed #503 for this and included my two ideas on how we could deal with it. We can prioritize at an upcoming meeting. I will not worry about Safari any further for this PR. |
OK, done in latest commit. By my reading of the above, that just leaves "each thumbnails/list display toggle should remember its own particular state" and "intermittent previews disappearing bug" for this PR. I will work on those when I can (and do initial review on "colourful ModFill" as well). |
OK, each display toggle should now have its own remembered state. |
Hmmm, I followed exactly your recipe and was able to get every sequence appearing in the search for |
OK, I've pushed my aspirational attempt to resolve the non-rendering thumbnails issue. Let me know if you're still able to produce the problem. If so, give me your best effort at a reliable recipe for producing it (or maybe just let me know that the recipe you already posted still makes it happen in your browser). If I still can't reproduce, we will have to try a joint debugging session sometime. |
Ah, I got it to happen in Chromium. Will investigate. |
Ugh. It's a well-established resource limit in all browsers, just happens to be significantly lower in Chromium than Firefox: A browser only supports some particular number of WebGL contexts, and currently each separate active WebGL p5.js sketch creates its own WebGL context. Some major re-engineering will be needed here. Some options we could pursue, in no particular order:
|
On the most current PR everything seems fixed except the WebGL limit. Maybe this PR turned up the webgl problem, since it involved so much playing with switchers, but it isn't really part of this PR's scope perhaps? I'll plan to finish reviewing the rest of the PR meanwhile. |
Here's a nice article about the precisely analogous problem for three.js. It offers two other possible solutions (translated to our circumstance):
I'm still at a bit of a loss as to which way to go here; all these options (except maybe the one about counting canvases and inserting a list view at the bottom with the ones that would go over the limit) seem like a lot of work. Definitely anyone with advice/thoughts please share! |
Well, maybe you could check whether you can also get it to happen in ui2 as it stands, and if so, then yes I agree it wasn't created by this PR, so we could file an issue for the problem and go ahead with wrapping this PR up with the issue outstanding. But doing something about the WebGL limit -- at the very least gracefully informing the visitor about the limitation and not just having an ugly frowny face which looks like our bug -- would be imperative for alpha. We could discuss just how completely we want to work around this limitation in alpha, and what strategy to use for the selected work around, at our next meeting. Let me know your thoughts on this. |
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.
typos
Well to add to the list of possibilities, we could "downgrade" turtle and histogram from WebGL for alpha and just not use WebGL until we sort that out. WebGL has been a bit of a headache, and it doesn't seem strictly necessary for those two visualizers. (I agree there may come some visualizers which really need it, but those two are essentially 2d still.) Just another idea for your long list... not necessarily the one I'm promoting. |
Yes, I hereby verify that. It happens on ui2. |
OK, do you want me to file an issue describing the problem and listing all the collected possible responses, and we'll consider it officially Not Part Of This PR? As to reverting visualizers to non-WebGL: A) On FactorHistogram, when I re-wrote the hatching code (because it turns out p5.brush doesn't actually support multiple different p5 instances, we had just been getting lucky while we didn't have too many previews), I almost did just that. I only didn't because sometimes pan/zoom is actually nice. But they are of pretty minor value for this visualizer, so I would have no real problem with reverting. B) However on turtle, the WebGL polyline object turns out to be a major performance improvement, and the pan/zoom/rotate are really useful, and they would be a big pain and I think much less performant to re-implement in ordinary drawing canvas. We could either investigate the svg version of p5, or just make an SVGVisualizer class, and probably get similar ease of implementation and performance on turtle, but then there would be the work of getting one or the other svg-based visualizer framework running properly. So given that there's noticeable work in maintaining the current really worthwhile level of functionality in Turtle with a non-WebGL solution, and that WebGL will very likely be truly valuable for other visualizers, currently existing or yet to be written, and enables 3D visualizers right now, I'm reluctant to pursue a "no WebGL for now" solution. Seems to me like that would be more work in the long run, since we will very likely want WebGL capabilities in at least the medium-term future if not sooner. |
Yeah let's do that. |
Oh, I forgot that was a WebGL feature! Ok, yes, we need WebGL. I'm convinced. |
Ok, got your typo fixes. At this point I've extensively tested this by hand and it is passing all automated tests on my machine, so we can make the webGL thing a separate issue (mark it meeting, for sure!), and I'm happy to merge this. |
…scope#500) * feat: Save a history of any kind of Sequences (not just OEIS) This partially resolves numberscope#430; the only portion remaining is to have a limit on the number of sequences that will show as thumbnails (the rest will show as a table or something like that). Also, testing for this commit revealed that the p5.brush addon does not work properly with multiple different sketches all updating asynchronously. Therefore, this commit removes p5.brush and provides a simple hatchRect() method in P5Visualizer (since hatching is all that p5.brush was being used for anyway). The current sequence is saved/updated in the list of remembered sequences when (a) the Scope view is first loaded; (b) the Scope view is closed; (c) a sequence or visualizer is selected in the SwitcherModal; or (d) the current specimen is saved to the Gallery. This commit removes the former list of OEIS IDs that was being saved in browser localStorage, as it is superseded by this sequence-saving feature. That removal means that the frontscope no longer (pre-)fetches a collection of OEIS sequences immediately on startup; they are loaded only as used. * fix: Improve circumstances under which sequence is saved to history Also save a sequence when the switcher is opened (so there is no need to save it when a new sequence is selected, since the switcher must have been opened in order for something else to be selected), and save the destination sequence any time a specimen card is selected. Also puts the mod information in the description of an OEIS sequence so that we don't end up with a bunch of identical-looking sequence cards. * feat: Add a list view to SpecimensGallery Also adds a simple toggle switch component for selecting how a given SpecimensGallery should be displayed. Makes the title and subtitle properties for a specimen card mandatory (as they are displayed in the table view). Adds the visualizer as well as the sequence to the subtitle for featured and saved specimens so that the table view would be more informative (and the extra information also shows up in the thumbnails view. This commit significantly reorganizes the HTML/CSS of a SpecimensGallery component to accomodate the toggling between the two views (not to mention the non-scrolling control to do that toggling). All of the same abilities (select a specimen, delete a specimen, link to OEIS) are present in the table view as in the thumbnails view. Canonicalizes the query string of a couple of the featured items, to prevent them from apparently creating duplicate saved sequences. * ui: several sequence history tweaks per discussion in numberscope#500 1) Renames table view to list view. 3) Remembers the last-used view in browser-local storage 4) Keeps undeleteable 'Formula: n' and 'Random from 0 to 9' at the head of the history list. Thereafter entries are in most- recently-used order. History defaults to the Beatty-sequence formula, OEIS primes, and OEIS VF numbers. 5) In the display toggle, both options are in numberscope grey, with the active one in the slightly heavier weight we use. The inactive one turns black on hover, indicating it can be clicked on. (As always, you can also click directly on the toggle switch.) * feat: MathML display for Formula sequences Adds an `htmlName` property (defaulting to `name`) to a Paramable objects. The idea is that the user interface is supposed to use the htmlName where possible to display the name of an element. Implements `htmlName` for Formula sequences, to contain the MathML rendering of their `formula` parameter. Adds a `v-safe-html` directive to Vue to facilitate using this generated MathML. Displays the `htmlName` using the new directive in the ParamEditor, SpecimenCard titles, and in the columns of the list view in a SpecimensGallery. Also adds the ability to tailor the heading of the first column in the list view, so that it reads "Visualizer" in the visualizer switcher, etc. * chore: dummy commit to grab CI snapshots * chore: New CI snapshots to reflect mathml formulas * ui: Try to improve switcher height calculation * ui: Always title the permanent Formula card 'Formula' * ui: Enable formula wrapping on chromium-based browsers * ui: Actually make any sequence with no query have special title * feat: Store preference for each gallery display style separately * fix: Attempt to head off non-rendering thumbnails * chore: fix comment typos
OK. Issue #504 created. Moving on from this point for the moment. |
* feat: Save a history of any kind of Sequences (not just OEIS) This partially resolves #430; the only portion remaining is to have a limit on the number of sequences that will show as thumbnails (the rest will show as a table or something like that). Also, testing for this commit revealed that the p5.brush addon does not work properly with multiple different sketches all updating asynchronously. Therefore, this commit removes p5.brush and provides a simple hatchRect() method in P5Visualizer (since hatching is all that p5.brush was being used for anyway). The current sequence is saved/updated in the list of remembered sequences when (a) the Scope view is first loaded; (b) the Scope view is closed; (c) a sequence or visualizer is selected in the SwitcherModal; or (d) the current specimen is saved to the Gallery. This commit removes the former list of OEIS IDs that was being saved in browser localStorage, as it is superseded by this sequence-saving feature. That removal means that the frontscope no longer (pre-)fetches a collection of OEIS sequences immediately on startup; they are loaded only as used. * fix: Improve circumstances under which sequence is saved to history Also save a sequence when the switcher is opened (so there is no need to save it when a new sequence is selected, since the switcher must have been opened in order for something else to be selected), and save the destination sequence any time a specimen card is selected. Also puts the mod information in the description of an OEIS sequence so that we don't end up with a bunch of identical-looking sequence cards. * feat: Add a list view to SpecimensGallery Also adds a simple toggle switch component for selecting how a given SpecimensGallery should be displayed. Makes the title and subtitle properties for a specimen card mandatory (as they are displayed in the table view). Adds the visualizer as well as the sequence to the subtitle for featured and saved specimens so that the table view would be more informative (and the extra information also shows up in the thumbnails view. This commit significantly reorganizes the HTML/CSS of a SpecimensGallery component to accomodate the toggling between the two views (not to mention the non-scrolling control to do that toggling). All of the same abilities (select a specimen, delete a specimen, link to OEIS) are present in the table view as in the thumbnails view. Canonicalizes the query string of a couple of the featured items, to prevent them from apparently creating duplicate saved sequences. * ui: several sequence history tweaks per discussion in #500 1) Renames table view to list view. 3) Remembers the last-used view in browser-local storage 4) Keeps undeleteable 'Formula: n' and 'Random from 0 to 9' at the head of the history list. Thereafter entries are in most- recently-used order. History defaults to the Beatty-sequence formula, OEIS primes, and OEIS VF numbers. 5) In the display toggle, both options are in numberscope grey, with the active one in the slightly heavier weight we use. The inactive one turns black on hover, indicating it can be clicked on. (As always, you can also click directly on the toggle switch.) * feat: MathML display for Formula sequences Adds an `htmlName` property (defaulting to `name`) to a Paramable objects. The idea is that the user interface is supposed to use the htmlName where possible to display the name of an element. Implements `htmlName` for Formula sequences, to contain the MathML rendering of their `formula` parameter. Adds a `v-safe-html` directive to Vue to facilitate using this generated MathML. Displays the `htmlName` using the new directive in the ParamEditor, SpecimenCard titles, and in the columns of the list view in a SpecimensGallery. Also adds the ability to tailor the heading of the first column in the list view, so that it reads "Visualizer" in the visualizer switcher, etc. * chore: dummy commit to grab CI snapshots * chore: New CI snapshots to reflect mathml formulas * ui: Try to improve switcher height calculation * ui: Always title the permanent Formula card 'Formula' * ui: Enable formula wrapping on chromium-based browsers * ui: Actually make any sequence with no query have special title * feat: Store preference for each gallery display style separately * fix: Attempt to head off non-rendering thumbnails * chore: fix comment typos
The two main changes in this PR are that (i) the collection of saved OEIS IDs is
replaced with a history list of arbitrary sequences that have been used, and (ii)
it is now possible to toggle between a thumbnail and list presentations of
collections of specimens.
Testing for this commit revealed that the p5.brush addon does not work
properly with multiple different sketches all updating asynchronously.
Therefore, this commit removes p5.brush and provides a simple hatchRect()
method in P5Visualizer (since hatching is all that p5.brush was being used
for anyway).
The current sequence is saved/updated in the list of remembered sequences
when (a) the Scope view is first loaded; (b) the Scope view is closed;
(c) the sequence or visualizer switcher is popped up; or (d) the
current specimen is saved to the Gallery. In addition, the when any
specimen is selected from a chooser, its sequence is saved/updated in the
history list as well.
The removal of the list of OEIS IDs means that the frontscope no longer
(pre-)fetches a collection of OEIS sequences immediately on startup; they
are loaded only as used. That change has pros (avoids a potential long
startup lag) and cons (later operations more likely to pause before completing).
Resolves #430.
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.