davidson16807 / tectonics.js

3d plate tectonics in your web browser
http://davidson16807.github.io/tectonics.js/
Creative Commons Attribution 4.0 International
200 stars 28 forks source link

Screenshot broken #10

Closed astrographer closed 6 years ago

astrographer commented 6 years ago

Looking over the latest version, I found the Screenshot button(and

shortcut) was no longer available. For my purposes, at least, this is a vital element of functionality.

MacOS 10.12.6 Firefox 56.0.2

davidson16807 commented 6 years ago

OK, reproduced. The button shows up but does nothing when I click. Error doesn't occur in Chrome. That's really weird but I suspect some problem with the recent move to Vue.js. Will look into it tonight. Thanks for letting me know.

davidson16807 commented 6 years ago

The screenshot button is actually two html tags: a button and an anchor within the button. When the user clicks the screenshot button, the event reaches the button first. The button uses Vue.js to configure the anchor so that it downloads the data. The event then propagates to the anchor, which does the actual downloading. It's done this way because only an anchor can download the file, but an event has to occur beforehand to configure the anchor.

That's how it works in theory. In practice, Vue.js doesn't update the data before the anchor downloads, at least not in Firefox. I tried forcing Vue to update the anchor using $forceUpdate(), but it still doesn't work. I'm not going to investigate this further. I'm falling back on jQuery to do the data update, as was done previously. I was trying to phase out jQuery, but it's apparent from this and other problems that Vue.js is not a complete replacement.

I still have to commit these fixes and push them to prod. More to come.

astrographer commented 6 years ago

Awesome. If I’m reading what you’ve written correctly, it sounds like this might also fix the next issue I was going to push your way, which was that the age, density, thickness and plates views don’t properly update while paused. I’ll have to see if that works out with the new commit.

Also, I’d like to see the “p” shortcut remain available for screenshots. In an earlier version, pushing the Screenshot button also shut down the program, so I got in the habit of simply pressing the “p” key which rendered without exiting program execution. It’s gotten to be a habit…

Waiting eagerly for the new commit, Colin

On Nov 9, 2017, at 1:06 PM, Carl Davidson notifications@github.com wrote:

The screenshot button is actually two html tags: a button and an anchor within the button. When the user clicks the screenshot button, the event reaches the button first. The button uses Vue.js to configure the anchor so that it downloads the data. The event then propagates to the anchor, which does the actual downloading. It's done this way because only an anchor can download the file, but an event has to occur beforehand to configure the anchor.

That's how it works in theory. In practice, Vue.js doesn't update the data before the anchor downloads, at least not in Firefox. I tried forcing Vue to update the anchor using $forceUpdate(), but it still doesn't work. I'm not going to investigate this further. I'm falling back on jQuery to do the data update, as was done previously. I was trying to phase out jQuery, but it's apparent from this and other problems that Vue.js is not a complete replacement.

I still have to commit these fixes and push them to prod. More to come.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/davidson16807/tectonics.js/issues/10#issuecomment-343291389, or mute the thread https://github.com/notifications/unsubscribe-auth/AGayL4H8Yv1NLnFDWzL7ivMbw9N4WFLvks5s02lWgaJpZM4QVY33.

astrographer commented 6 years ago

I don't know what the advantages are to Vue.js, but would this help?

https://forum.vuejs.org/t/component-not-updating-when-data-changes/3163/3

On Nov 9, 2017, at 1:06 PM, Carl Davidson notifications@github.com wrote:

The screenshot button is actually two html tags: a button and an anchor within the button. When the user clicks the screenshot button, the event reaches the button first. The button uses Vue.js to configure the anchor so that it downloads the data. The event then propagates to the anchor, which does the actual downloading. It's done this way because only an anchor can download the file, but an event has to occur beforehand to configure the anchor.

That's how it works in theory. In practice, Vue.js doesn't update the data before the anchor downloads, at least not in Firefox. I tried forcing Vue to update the anchor using $forceUpdate(), but it still doesn't work. I'm not going to investigate this further. I'm falling back on jQuery to do the data update, as was done previously. I was trying to phase out jQuery, but it's apparent from this and other problems that Vue.js is not a complete replacement.

I still have to commit these fixes and push them to prod. More to come.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

davidson16807 commented 6 years ago

The fix is out. Go check it.

The screenshot bug is unrelated to the pause bug. I'd file a separate issue for that.

The pause bug has more to do with how the ScalarHeatDisplay class is defined within the "View" layer (the "View" layer is completely different from "Vue". Yeah, I know that's confusing, but that's really the only way I can name them. I describe them on the wiki)

Vue.js is a "Model-View-Control" framework for javascript. It's like React.js or Angular.js, if that means anything. React and Angular are more popular, but they're way too bloated and opinionated and I'm sick of working with them.

Vue.js is much more lightweight, and it plays nice with jQuery so it's easy for tectonics.js to migrate towards. I like it. I'll have to write a blog post about it some time. Having a Model-View-Control framework really simplifies development. It's what made it so easy to implement the "Arrow" dropdown.

It still has problems, like we see here, but the community isn't so militant to assert you can't use jQuery workarounds when it fails.

I didn't realize anyone found the "p" shortcut handy so I didn't bother to reimplement it during the switch to Vue. I'll have to add it back.

davidson16807 commented 6 years ago

keyboard shortcuts have been reimplemented

davidson16807 commented 6 years ago

Alright, I don't expect it should be a problem again. Feel free to open a new issue if otherwise.