srid / ema

Change-aware static site generator for Haskell programmers
https://ema.srid.ca
GNU Affero General Public License v3.0
117 stars 9 forks source link

Ema scrolls to the anchor in every DOM patch #88

Open luavreis opened 2 years ago

luavreis commented 2 years ago

This is just for keeping track of the discussion in https://github.com/srid/ema/pull/86#issuecomment-1044854353. (if the URL being displayed has an anchor, every time the page is patched the client would scroll to the anchor again).

Proposed fix (to the current, non-multisite ema) is:

 src/Ema/Server.hs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/Ema/Server.hs b/src/Ema/Server.hs
index cb98105..882f188 100644
--- a/src/Ema/Server.hs
+++ b/src/Ema/Server.hs
@@ -278,6 +278,8 @@ wsClientShim =
         function init(reconnecting) {
           // The route current DOM is displaying
           let routeVisible = document.location.pathname;
+          // If client should scroll to anchor on next DOM patch
+          let pendingHashChange = false;

           const verb = reconnecting ? "Reopening" : "Opening";
           console.log(`ema: $${verb} conn $${wsUrl} ...`);
@@ -300,6 +302,7 @@ wsClientShim =
           function switchRoute(path, hash = "") {
              console.log(`ema: → Switching to $${path + hash}`);
              window.history.pushState({}, "", path + hash);
+             if (hash) { pendingHashChange = true };
              sendObservePath(path);
           }

@@ -358,11 +361,12 @@ wsClientShim =
                 window.scrollTo({ top: 0});
                 routeVisible = document.location.pathname;
               }
-              if (window.location.hash) {
+              if (pendingHashChange && window.location.hash) {
                 var el = document.querySelector(window.location.hash);
                 if (el !== null) {
                   el.scrollIntoView({ behavior: 'smooth' });
                 }
+                pendingHashChange = false;
               }
               watchCurrentRoute();
             };
-- 
2.35.1
luavreis commented 2 years ago

@srid recently I'd been facing other issues related to route switching, so I was wondering, what is the reason to do the route switching via JavaScript instead of leaving it to the browser?

Right now I'm using https://github.com/lucasvreis/ema/commit/13dfbeca8d5bda5702378b292d54d057c98df03a that simply removes the switchRoute code, and now route changes are handled by the browser as usual. The server will disconnect and then reconnect, but I don't see a big issue in this. When you are editing a page, hot reload works as usual. And I think this has advantages:

srid commented 2 years ago

@lucasvreis Fast page load is the main reason (websocket connection is smoother; also morphdom patches only what's changed) that I can recall. I just tried out bin/run examples with switchRoute commented out and there is indeed a noticeable delay (along with the orange indicator flashing on top-left) between page transitions. EDIT: with switchRoute disabled, it is worth keeping in mind that the websocket connection is reestablished on each page switch which adds to the delay.

I am not very familiar with the JS reliability issues you are facing, but I can see how that can be problematic with switchRoute even though in my experience it has been fine. Perhaps we can provide an option to disable switchRoute entirely (with it enabled by default). Say a CLI argument like run --port=8080 --disable-switchRoute (which would also carry over to emanote). How does that sound?

srid commented 10 months ago

Perhaps we can provide an option to disable switchRoute entirely (with it enabled by default). Say a CLI argument like run --port=8080 --disable-switchRoute (which would also carry over to emanote). How does that sound?

Specific proposal somewhat related to this: https://github.com/srid/ema/issues/159