Skip to content
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

URL out-of-sync bug in parameter handling in ui2 #403

Closed
katestange opened this issue Jul 25, 2024 · 4 comments
Closed

URL out-of-sync bug in parameter handling in ui2 #403

katestange opened this issue Jul 25, 2024 · 4 comments
Labels
question Further information is requested ui Something having to do with the user interface

Comments

@katestange
Copy link
Member

There's an important bug that I'm having to handle in enhancing turtle, but should properly be fixed in ui2, not in turtle. Here's the situation:

Suppose that inside checkParameters() a parameter should be modified to conform. For example, some visualizers will stop you from putting a "number of terms" that exceeds this.seq.last - this.seq.first (quite reasonably). It would be nice to be able to simply replace a large number entered by the user with the maximum allowed by the sequence, and display this updated value in the parameter pane right away. Note that no default value here is simultaneously reasonable and safe, as many OEIS sequences have very few terms. So if you don't want the visualizer to load up in a default invalid state on such sequences, you want to just update the number of terms (including the default) if it is too large immediately to the sequence's allowable max. This is also a favour to the user, who can't see the max in the UI at the moment. So this is something we probably want to do, and I find myself wanting to do in Turtle.

Now, this check can't be performed earlier than checkParameters() since it needs access to this.seq. So it happens inside checkParameters(), and then you have to call refreshParams() to display the modified value to the user, presumably at the start of presketch(). This works.

Except that in this scenario the URL doesn't update with the correction in refreshParams() and instead shows whatever too-large value the user typed (it appears as the user is typing). So then the URL is out of sync with the visualizer's actual internal state, and hard reloading or sharing the URL will cause a crash (or whatever the bad value will cause) because a reload or fresh load doesn't seem to re-run checkParameters().

@katestange katestange added bug ui Something having to do with the user interface labels Jul 25, 2024
@gwhitney
Copy link
Collaborator

The description is totally understandable, and I think Chaos is already doing something similar with replacing its first and last indices with those of the sequence when they are out of bounds. Can the bug be exercised with existing Chaos visualizer? Generally speaking, I am happy to work on this, I just need an explicit "recipe" for making the bug occur.

@gwhitney
Copy link
Collaborator

Ah, I see, it happens with A371958 under the current Chaos visualizer. That sequence only has 15 terms, so the "last" parameter shows 15. If you type 0 after that in the input field, the URL changes to include last=150 even though the parameter box still shows 15. OK, I will take a look.

gwhitney added a commit to gwhitney/frontscope that referenced this issue Jul 29, 2024
   In order to be sure that parameter values have their "final" values
   in the case of an update (e.g., by calling the Paramable validate()
   method), it is necessary for a cascade of functions to be made async.
   This commit makes those changes and then awaits the validate() in the
   user interface, so that the URL won't be updated until all of the
   consequences of validate() have occurred (which may, for example,
   involve changing the parameter values, perhaps even the one that
   originally had its tentative value modified, necessitating the validate()
   call).

   Resolves numberscope#403.
katestange added a commit that referenced this issue Aug 15, 2024
* refactor: move the OEIS search bar into its own component.

* doc: Basic Modfill -> two OEIS-based Specimens in Gallery

* feat: Dummy search results that pop up on input

* feat: Call a dummy search cmd on input

* feat: list of links and labels for search results

* feat: Clicking on a result adds it

* fix: When you click on link in results don't also add

* feat: Actually search the OEIS.

* feat: Cache search results and reopen search on click.

* ui: Layout and caption changes per design discussions

* fix: Never cache OEIS sequences with infinite indices
   This PR attempts to resolve #407. I added a half-dozen OEIS
   sequences as rapidly as I could and didn't get any errors.
   So hopefully this has squashed the problem, but it's hard to
   be certain.

* fix: Don't update URL until after parameter values have settled
   In order to be sure that parameter values have their "final" values
   in the case of an update (e.g., by calling the Paramable validate()
   method), it is necessary for a cascade of functions to be made async.
   This commit makes those changes and then awaits the validate() in the
   user interface, so that the URL won't be updated until all of the
   consequences of validate() have occurred (which may, for example,
   involve changing the parameter values, perhaps even the one that
   originally had its tentative value modified, necessitating the validate()
   call).

   Resolves #403.

* ui: show finger cursor over search results

* make oeis.org link open new tab

* info icon for info in search results

* ui: click off list of OEIS search results closes it

* fix: Restore sequences adding when you click on them in result list.

* fix: Various fixes to OEIS search bar/results per meeting

  - Return to the OEIS ID being a link but add the Wikipedia
    "external link" icon
  - Highlight the description of each sequence as you mouse over it,
    to provide clue you can click on it (to add as a sequence). The
    color used is a lightened version of the "primary" Numberscope color.
  - Display a message when there are no sequences to show as a result
    of a search. Note this can occur because there are _too many_ matches,
    or _no_ matches, and there is no way to tell which is the case from
    the OEIS api response, unfortunately. The message reflects this.
  - Fixed bug that results were being cached under the wrong search term
    if the searcher typed more while the search was executing.
  - Make sure that enter/clicking on magnifier always fires search

* fix: Don't make switcher cards for dummy message IDs

* fix: Really make sure magnifier puts/keeps search results up; reword message

* fix: catch errors in OEIS searching and report a 'soft' failure

---------

Co-authored-by: Kate Stange <[email protected]>
@gwhitney gwhitney added the question Further information is requested label Aug 28, 2024
@gwhitney
Copy link
Collaborator

@katestange would you be able to check if this has now been resolved in ui2, or if you can still reproduce it in this branch? If it seems resolved, feel free to close this issue; if not, please add any further details needed here, or at least a verification that it still exists. Marking this for alpha as it seems it is/was a bug in the new code.

@katestange
Copy link
Member Author

This appears to be resolved in ui2, so I'm going to close the issue.

katestange added a commit to katestange/frontscope that referenced this issue Aug 31, 2024
* refactor: move the OEIS search bar into its own component.

* doc: Basic Modfill -> two OEIS-based Specimens in Gallery

* feat: Dummy search results that pop up on input

* feat: Call a dummy search cmd on input

* feat: list of links and labels for search results

* feat: Clicking on a result adds it

* fix: When you click on link in results don't also add

* feat: Actually search the OEIS.

* feat: Cache search results and reopen search on click.

* ui: Layout and caption changes per design discussions

* fix: Never cache OEIS sequences with infinite indices
   This PR attempts to resolve numberscope#407. I added a half-dozen OEIS
   sequences as rapidly as I could and didn't get any errors.
   So hopefully this has squashed the problem, but it's hard to
   be certain.

* fix: Don't update URL until after parameter values have settled
   In order to be sure that parameter values have their "final" values
   in the case of an update (e.g., by calling the Paramable validate()
   method), it is necessary for a cascade of functions to be made async.
   This commit makes those changes and then awaits the validate() in the
   user interface, so that the URL won't be updated until all of the
   consequences of validate() have occurred (which may, for example,
   involve changing the parameter values, perhaps even the one that
   originally had its tentative value modified, necessitating the validate()
   call).

   Resolves numberscope#403.

* ui: show finger cursor over search results

* make oeis.org link open new tab

* info icon for info in search results

* ui: click off list of OEIS search results closes it

* fix: Restore sequences adding when you click on them in result list.

* fix: Various fixes to OEIS search bar/results per meeting

  - Return to the OEIS ID being a link but add the Wikipedia
    "external link" icon
  - Highlight the description of each sequence as you mouse over it,
    to provide clue you can click on it (to add as a sequence). The
    color used is a lightened version of the "primary" Numberscope color.
  - Display a message when there are no sequences to show as a result
    of a search. Note this can occur because there are _too many_ matches,
    or _no_ matches, and there is no way to tell which is the case from
    the OEIS api response, unfortunately. The message reflects this.
  - Fixed bug that results were being cached under the wrong search term
    if the searcher typed more while the search was executing.
  - Make sure that enter/clicking on magnifier always fires search

* fix: Don't make switcher cards for dummy message IDs

* fix: Really make sure magnifier puts/keeps search results up; reword message

* fix: catch errors in OEIS searching and report a 'soft' failure

---------

Co-authored-by: Kate Stange <[email protected]>
katestange added a commit to katestange/frontscope that referenced this issue Oct 10, 2024
* refactor: move the OEIS search bar into its own component.

* doc: Basic Modfill -> two OEIS-based Specimens in Gallery

* feat: Dummy search results that pop up on input

* feat: Call a dummy search cmd on input

* feat: list of links and labels for search results

* feat: Clicking on a result adds it

* fix: When you click on link in results don't also add

* feat: Actually search the OEIS.

* feat: Cache search results and reopen search on click.

* ui: Layout and caption changes per design discussions

* fix: Never cache OEIS sequences with infinite indices
   This PR attempts to resolve numberscope#407. I added a half-dozen OEIS
   sequences as rapidly as I could and didn't get any errors.
   So hopefully this has squashed the problem, but it's hard to
   be certain.

* fix: Don't update URL until after parameter values have settled
   In order to be sure that parameter values have their "final" values
   in the case of an update (e.g., by calling the Paramable validate()
   method), it is necessary for a cascade of functions to be made async.
   This commit makes those changes and then awaits the validate() in the
   user interface, so that the URL won't be updated until all of the
   consequences of validate() have occurred (which may, for example,
   involve changing the parameter values, perhaps even the one that
   originally had its tentative value modified, necessitating the validate()
   call).

   Resolves numberscope#403.

* ui: show finger cursor over search results

* make oeis.org link open new tab

* info icon for info in search results

* ui: click off list of OEIS search results closes it

* fix: Restore sequences adding when you click on them in result list.

* fix: Various fixes to OEIS search bar/results per meeting

  - Return to the OEIS ID being a link but add the Wikipedia
    "external link" icon
  - Highlight the description of each sequence as you mouse over it,
    to provide clue you can click on it (to add as a sequence). The
    color used is a lightened version of the "primary" Numberscope color.
  - Display a message when there are no sequences to show as a result
    of a search. Note this can occur because there are _too many_ matches,
    or _no_ matches, and there is no way to tell which is the case from
    the OEIS api response, unfortunately. The message reflects this.
  - Fixed bug that results were being cached under the wrong search term
    if the searcher typed more while the search was executing.
  - Make sure that enter/clicking on magnifier always fires search

* fix: Don't make switcher cards for dummy message IDs

* fix: Really make sure magnifier puts/keeps search results up; reword message

* fix: catch errors in OEIS searching and report a 'soft' failure

---------

Co-authored-by: Kate Stange <[email protected]>
katestange added a commit to katestange/frontscope that referenced this issue Oct 13, 2024
* refactor: move the OEIS search bar into its own component.

* doc: Basic Modfill -> two OEIS-based Specimens in Gallery

* feat: Dummy search results that pop up on input

* feat: Call a dummy search cmd on input

* feat: list of links and labels for search results

* feat: Clicking on a result adds it

* fix: When you click on link in results don't also add

* feat: Actually search the OEIS.

* feat: Cache search results and reopen search on click.

* ui: Layout and caption changes per design discussions

* fix: Never cache OEIS sequences with infinite indices
   This PR attempts to resolve numberscope#407. I added a half-dozen OEIS
   sequences as rapidly as I could and didn't get any errors.
   So hopefully this has squashed the problem, but it's hard to
   be certain.

* fix: Don't update URL until after parameter values have settled
   In order to be sure that parameter values have their "final" values
   in the case of an update (e.g., by calling the Paramable validate()
   method), it is necessary for a cascade of functions to be made async.
   This commit makes those changes and then awaits the validate() in the
   user interface, so that the URL won't be updated until all of the
   consequences of validate() have occurred (which may, for example,
   involve changing the parameter values, perhaps even the one that
   originally had its tentative value modified, necessitating the validate()
   call).

   Resolves numberscope#403.

* ui: show finger cursor over search results

* make oeis.org link open new tab

* info icon for info in search results

* ui: click off list of OEIS search results closes it

* fix: Restore sequences adding when you click on them in result list.

* fix: Various fixes to OEIS search bar/results per meeting

  - Return to the OEIS ID being a link but add the Wikipedia
    "external link" icon
  - Highlight the description of each sequence as you mouse over it,
    to provide clue you can click on it (to add as a sequence). The
    color used is a lightened version of the "primary" Numberscope color.
  - Display a message when there are no sequences to show as a result
    of a search. Note this can occur because there are _too many_ matches,
    or _no_ matches, and there is no way to tell which is the case from
    the OEIS api response, unfortunately. The message reflects this.
  - Fixed bug that results were being cached under the wrong search term
    if the searcher typed more while the search was executing.
  - Make sure that enter/clicking on magnifier always fires search

* fix: Don't make switcher cards for dummy message IDs

* fix: Really make sure magnifier puts/keeps search results up; reword message

* fix: catch errors in OEIS searching and report a 'soft' failure

---------

Co-authored-by: Kate Stange <[email protected]>
gwhitney added a commit that referenced this issue Jan 20, 2025
* refactor: move the OEIS search bar into its own component.

* doc: Basic Modfill -> two OEIS-based Specimens in Gallery

* feat: Dummy search results that pop up on input

* feat: Call a dummy search cmd on input

* feat: list of links and labels for search results

* feat: Clicking on a result adds it

* fix: When you click on link in results don't also add

* feat: Actually search the OEIS.

* feat: Cache search results and reopen search on click.

* ui: Layout and caption changes per design discussions

* fix: Never cache OEIS sequences with infinite indices
   This PR attempts to resolve #407. I added a half-dozen OEIS
   sequences as rapidly as I could and didn't get any errors.
   So hopefully this has squashed the problem, but it's hard to
   be certain.

* fix: Don't update URL until after parameter values have settled
   In order to be sure that parameter values have their "final" values
   in the case of an update (e.g., by calling the Paramable validate()
   method), it is necessary for a cascade of functions to be made async.
   This commit makes those changes and then awaits the validate() in the
   user interface, so that the URL won't be updated until all of the
   consequences of validate() have occurred (which may, for example,
   involve changing the parameter values, perhaps even the one that
   originally had its tentative value modified, necessitating the validate()
   call).

   Resolves #403.

* ui: show finger cursor over search results

* make oeis.org link open new tab

* info icon for info in search results

* ui: click off list of OEIS search results closes it

* fix: Restore sequences adding when you click on them in result list.

* fix: Various fixes to OEIS search bar/results per meeting

  - Return to the OEIS ID being a link but add the Wikipedia
    "external link" icon
  - Highlight the description of each sequence as you mouse over it,
    to provide clue you can click on it (to add as a sequence). The
    color used is a lightened version of the "primary" Numberscope color.
  - Display a message when there are no sequences to show as a result
    of a search. Note this can occur because there are _too many_ matches,
    or _no_ matches, and there is no way to tell which is the case from
    the OEIS api response, unfortunately. The message reflects this.
  - Fixed bug that results were being cached under the wrong search term
    if the searcher typed more while the search was executing.
  - Make sure that enter/clicking on magnifier always fires search

* fix: Don't make switcher cards for dummy message IDs

* fix: Really make sure magnifier puts/keeps search results up; reword message

* fix: catch errors in OEIS searching and report a 'soft' failure

---------

Co-authored-by: Kate Stange <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested ui Something having to do with the user interface
Projects
None yet
Development

No branches or pull requests

2 participants