Instead of storing only scroll position, store "page state"

This should work the same as before. Next step: store image heights and set them on <img> tags as early as possible in the page's loading process.
AndrewRadev · Mar 29, 2020 · 7fe3714 · 7fe3714
1 parent 0a4a6a0
commit 7fe3714
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 23 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -25,6 +25,8 @@ log            = "0.4"
 notify         = "4.0.15"
 pathbuftools   = "0.1.2"
 pulldown-cmark = "0.7.0"
+serde          = { version = "1.0", features = ["derive"] }
+serde_json     = "1.0"
 structopt      = { version = "0.3.11", default-features = false }
 tempfile       = "3.1.0"
 webkit2gtk     = "0.7"

diff --git a/res/js/main.js b/res/js/main.js
@@ -2,12 +2,16 @@
 document.addEventListener('readystatechange', function() {
   if (document.readyState == 'interactive') {
     const title = document.querySelector('title');
-    window.scroll(0, title.innerHTML);
+    const page_state = JSON.parse(title.innerHTML);
+    window.scroll(0, page_state.scroll_top);
   }
 });
 
 // Store scroll position on scroll:
 window.addEventListener('scroll', function() {
   let title = document.querySelector('title');
-  title.innerHTML = window.pageYOffset.toString();
+  const page_state = JSON.parse(title.innerHTML);
+  page_state.scroll_top = window.pageYOffset;
+
+  title.innerHTML = JSON.stringify(page_state);
 });
diff --git a/res/layout.html b/res/layout.html
@@ -2,7 +2,7 @@
   <head>
     <meta charset="utf8" />
 
-    <title>{scroll_top}</title>
+    <title>{page_state}</title>
 
     <link rel="stylesheet" href="github.css" type="text/css" media="screen" />
     <link rel="stylesheet" href="main.css" type="text/css" media="screen" />

diff --git a/src/assets.rs b/src/assets.rs
@@ -7,20 +7,35 @@
 //! For the other assets, it means the HTML can refer to local files instead of embedding the
 //! contents as `<script>` and `<style>` tags, making the output easier to read and debug.
 
-use std::io;
+use std::collections::HashMap;
 use std::fs;
+use std::io;
 use std::path::PathBuf;
 use std::rc::Rc;
 
 use anyhow::anyhow;
 use dirs::home_dir;
-use tempfile::{tempdir, TempDir};
 use log::{debug, warn};
+use serde::{Serialize, Deserialize};
+use tempfile::{tempdir, TempDir};
 
 const MAIN_JS:    &str = include_str!("../res/js/main.js");
 const MAIN_CSS:   &str = include_str!("../res/style/main.css");
 const GITHUB_CSS: &str = include_str!("../res/style/github.css");
 
+/// The client-side state of the page as the user's interacted with it. Currently, includes the
+/// scroll position and the heights of images on the page (Note: not yet implemented), so that
+/// reloading doesn't change the viewport.
+///
+#[derive(Debug, Default, Serialize, Deserialize)]
+pub struct PageState {
+    /// Scroll position of the page.
+    pub scroll_top: f64,
+
+    /// A dictionary of all the heights of images in the page, keyed by src URL.
+    pub image_heights: HashMap<String, f64>,
+}
+
 /// A container for static assets.
 ///
 /// Has a temporary directory where it builds everything. Internally reference-counted, so clones
@@ -59,23 +74,29 @@ impl Assets {
     ///
     /// Returns the path to the generated HTML file, or an error.
     ///
-    pub fn build(&self, html: &str, scroll_top: f64) -> anyhow::Result<PathBuf> {
+    pub fn build(&self, html: &str, page_state: &PageState) -> anyhow::Result<PathBuf> {
         let temp_dir = self.temp_dir.clone().
             ok_or_else(|| anyhow!("TempDir deleted, there might be a synchronization error"))?;
 
         let home_path = home_dir().
             map(|p| p.display().to_string()).
             unwrap_or_else(String::new);
 
+        let json_state = serde_json::to_string(page_state).
+            unwrap_or_else(|e| {
+                warn!("Couldn't build JSON state from {:?}: {:?}", page_state, e);
+                String::from("{}")
+            });
+
         debug!("Building HTML:");
         debug!(" > home_path  = {}", home_path);
-        debug!(" > scroll_top = {}", scroll_top);
+        debug!(" > page_state = {:?}", json_state);
 
         let page = format! {
             include_str!("../res/layout.html"),
             home_path=home_path,
             body=html,
-            scroll_top=scroll_top,
+            page_state=json_state,
         };
 
         let output_path = temp_dir.path().join("output.html");

diff --git a/src/ui.rs b/src/ui.rs
@@ -10,7 +10,7 @@ use log::{debug, warn};
 use pathbuftools::PathBufTools;
 use webkit2gtk::{WebContext, WebView, WebViewExt};
 
-use crate::assets::Assets;
+use crate::assets::{Assets, PageState};
 
 /// Events that trigger UI changes.
 ///
@@ -83,11 +83,16 @@ impl App {
     }
 
     fn load_html(&mut self, html: &str) -> anyhow::Result<()> {
-        let scroll_top = self.webview.get_title().
-            and_then(|t| t.parse::<f64>().ok()).
-            unwrap_or(0.0);
-
-        let output_path = self.assets.build(html, scroll_top)?;
+        let page_state = match self.webview.get_title() {
+            Some(t) => {
+                serde_json::from_str(t.as_str()).unwrap_or_else(|e| {
+                    warn!("Failed to get page state from {}: {:?}", t, e);
+                    PageState::default()
+                })
+            },
+            None => PageState::default(),
+        };
+        let output_path = self.assets.build(html, &page_state)?;
 
         debug!("Loading HTML:");
         debug!(" > output_path = {}", output_path.display());

diff --git a/tests/test_assets.rs b/tests/test_assets.rs
@@ -1,5 +1,5 @@
 use std::fs;
-use quickmd::assets::Assets;
+use quickmd::assets::{Assets, PageState};
 
 macro_rules! assert_contains {
     ($haystack:expr, $needle:expr) => {
@@ -29,7 +29,7 @@ fn test_building_a_file_with_assets_includes_the_given_html() {
     let assets = Assets::init().unwrap();
     let html = "<h1>Example</h1>";
 
-    let path = assets.build(html, 0.0).unwrap();
+    let path = assets.build(html, &PageState::default()).unwrap();
 
     assert!(fs::read_to_string(&path).unwrap().contains(html));
     assert!(fs::read_to_string(&path).unwrap().contains("main.js"));
@@ -39,7 +39,7 @@ fn test_building_a_file_with_assets_includes_the_given_html() {
 #[test]
 fn test_building_a_file_with_assets_includes_main_static_files() {
     let assets = Assets::init().unwrap();
-    let path = assets.build("", 0.0).unwrap();
+    let path = assets.build("", &PageState::default()).unwrap();
 
     assert!(fs::read_to_string(&path).unwrap().contains("main.js"));
     assert!(fs::read_to_string(&path).unwrap().contains("main.css"));
@@ -48,13 +48,13 @@ fn test_building_a_file_with_assets_includes_main_static_files() {
 #[test]
 fn test_building_a_file_with_assets_includes_scroll_position_as_the_title() {
     let assets = Assets::init().unwrap();
-    let path = assets.build("", 100.5).unwrap();
+    let page_state = PageState { scroll_top: 100.5, ..PageState::default() };
+    let path = assets.build("", &page_state).unwrap();
 
     // Yes, it's included as the title, it's kind of dumb, but incredibly easy compared to the
     // alternative.
-    assert_contains!(fs::read_to_string(&path).unwrap(), "<title>100.5</title>");
-
-    // Number ends up rounded to an integer if it's a .0 float:
-    let path = assets.build("", 40.0).unwrap();
-    assert_contains!(fs::read_to_string(&path).unwrap(), "<title>40</title>");
+    assert_contains!(
+        fs::read_to_string(&path).unwrap(),
+        r#"<title>{"scroll_top":100.5,"image_heights":{}}</title>"#
+    );
 }