arnoudkooi
commented
7 months ago As said in the PR, seems like a nice addition.
A few remarks:
- I have changed to use the Fetch API, the snuFetch is a utility function for this.
- The distinction between the class and code outside the class is not clear to me, why not all in that class?
- Public functions, classes, variables and elements I want to prefix with snu (This is only on this page, but want to be consistent going forward)
- I'd rather have the HTML created with createElement, rather than string concatenation.
- Maybe include more info in the select, like execution time or # of characters in the script
- Limit to 50 entries
- Hide the button and pre before selection is made
- Maybe only load old scripts when select is clicked
- Should work in sys.scripts.modern.do in Washington as well, I'll implement the other updates here later.
These are some first thoughts, let me know if you want to proceed with it. Maybe you can also check in via apps@arnoudkooi.com or a different channel.
ewatch
Hey @arnoudkooi,
what do you think about the idea to show the script execution history in the background script window to load prior executed scripts by the user again to easily execute them again.
Pull Request closed by you for this was already here: #466 Let me know if you want to integrate it, want to have some changes before or have improvement suggestions.
Best regards, Jan