google / blockly-games

Games for tomorrow's programmers.
https://blockly.games/
Apache License 2.0
1.29k stars 605 forks source link

Refactoring common ajax request code into storage. #132

Closed moniika closed 4 years ago

NeilFraser commented 4 years ago

Thanks for the updates. My only question is regarding the usage of BlocklyStorage.makeFormRequest. Currently it is only used in lib-gallery.

A quick (but incomplete) browse through the 'pond' branch doesn't show any other usages of this function. If it will only be used by the gallery, then it should be moved to lib-gallery.

moniika commented 4 years ago

Thanks for the updates. My only question is regarding the usage of BlocklyStorage.makeFormRequest. Currently it is only used in lib-gallery.

A quick (but incomplete) browse through the 'pond' branch doesn't show any other usages of this function. If it will only be used by the gallery, then it should be moved to lib-gallery.

We would be using it in the pond-multi branch, though I'm not sure whether the need would be permanent. We make use of forms for request but its currently for testing purposes. I lean towards leaving it where it is now and moving it later if it turns out its definitely not needed.

NeilFraser commented 4 years ago

We would be using it in the pond-multi branch, though I'm not sure whether the need would be permanent. We make use of forms for request but its currently for testing purposes. I lean towards leaving it where it is now and moving it later if it turns out its definitely not needed.

My concern is that by placing it in storage.js, this function is being sent for every single game, even though most of them don't use it. Furthermore, storage.js is not compressed. And storage.js is a blocking load at the beginning. This adds 1KB to every page.

The less we have in storage.js, the better.

moniika commented 4 years ago

We would be using it in the pond-multi branch, though I'm not sure whether the need would be permanent. We make use of forms for request but its currently for testing purposes. I lean towards leaving it where it is now and moving it later if it turns out its definitely not needed.

My concern is that by placing it in storage.js, this function is being sent for every single game, even though most of them don't use it. Furthermore, storage.js is not compressed. And storage.js is a blocking load at the beginning. This adds 1KB to every page.

The less we have in storage.js, the better.

Makes sense. I've moved it to lib-gallery.