From 05e9eb38b98b4fb65d4493fcb060a0b9b77e839a Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 15:22:59 -0500 Subject: [PATCH 01/10] Fill in image link to lightbox (lightbox not working) Related to https://github.com/vector-im/hydrogen-web/issues/677 Before: ```
``` After: ```
Friction_between_surfaces.jpeg
``` --- .eslintrc.json | 2 +- shared/hydrogen-vm-render-script.js | 7 ++++- shared/lib/in-memory-history.js | 46 +++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 shared/lib/in-memory-history.js diff --git a/.eslintrc.json b/.eslintrc.json index 70f28763..d7dcb5d9 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -5,7 +5,7 @@ "node": true }, "parserOptions": { - "ecmaVersion": 2018, + "ecmaVersion": 2022, "sourceType": "script" }, "plugins": ["node"], diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index d47b4c50..b62124e9 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -23,6 +23,7 @@ const { const ArchiveView = require('matrix-public-archive-shared/ArchiveView'); const RightPanelContentView = require('matrix-public-archive-shared/RightPanelContentView'); const MatrixPublicArchiveURLCreator = require('matrix-public-archive-shared/lib/url-creator'); +const InMemoryHistory = require('matrix-public-archive-shared/lib/in-memory-history'); const fromTimestamp = window.matrixPublicArchiveContext.fromTimestamp; assert(fromTimestamp); @@ -87,11 +88,15 @@ async function mountHydrogen() { }); const navigation = createNavigation(); + const inMemoryHistory = new InMemoryHistory(); platform.setNavigation(navigation); const urlRouter = createRouter({ navigation: navigation, - history: platform.history, + //history: platform.history, + history: inMemoryHistory, }); + // Populate the `Navigation` with segments to work from + urlRouter.tryRestoreLastUrl(); // We use the timeline to setup the relations between entries const timeline = new Timeline({ diff --git a/shared/lib/in-memory-history.js b/shared/lib/in-memory-history.js new file mode 100644 index 00000000..965efda7 --- /dev/null +++ b/shared/lib/in-memory-history.js @@ -0,0 +1,46 @@ +'use strict'; + +const { BaseObservableValue } = require('hydrogen-view-sdk'); + +// ex. https://hydrogen.element.io/#/session/123/room/!abc:matrix.org/lightbox/$abc +// and the hash is handling the `/session/123/room/!abc:matrix.org/lightbox/$abc` part +class InMemoryHistory extends BaseObservableValue { + #hash = '#/session/123/room/!abc:matrix.org'; + + get() { + return this.#hash; + } + + /** does not emit */ + replaceUrlSilently(url) { + this.#hash = url; + } + + /** does not emit */ + pushUrlSilently(url) { + this.#hash = url; + } + + pushUrl(url) { + this.#hash = url; + } + + urlAsPath(url) { + if (url.startsWith('#')) { + return url.substr(1); + } else { + return url; + } + } + + pathAsUrl(path) { + return `#${path}`; + } + + getLastUrl() { + //throw new Error('Not implemented in InMemoryHistory'); + return this.#hash; + } +} + +module.exports = InMemoryHistory; From 16d503744901b8bac88319595554a0d2c18926ce Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 15:38:03 -0500 Subject: [PATCH 02/10] Use actual roomId --- shared/hydrogen-vm-render-script.js | 2 +- shared/lib/in-memory-history.js | 24 ++++++++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index b62124e9..5083afb7 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -88,7 +88,7 @@ async function mountHydrogen() { }); const navigation = createNavigation(); - const inMemoryHistory = new InMemoryHistory(); + const inMemoryHistory = new InMemoryHistory(roomData.id); platform.setNavigation(navigation); const urlRouter = createRouter({ navigation: navigation, diff --git a/shared/lib/in-memory-history.js b/shared/lib/in-memory-history.js index 965efda7..879b3188 100644 --- a/shared/lib/in-memory-history.js +++ b/shared/lib/in-memory-history.js @@ -1,11 +1,27 @@ 'use strict'; const { BaseObservableValue } = require('hydrogen-view-sdk'); - -// ex. https://hydrogen.element.io/#/session/123/room/!abc:matrix.org/lightbox/$abc -// and the hash is handling the `/session/123/room/!abc:matrix.org/lightbox/$abc` part +const assert = require('./assert'); + +// A Hydrogen History implementation that doesn't touch the URL. This way +// Hydrogen can still route internally while keeping our external facing URL the +// same. +// +// ex. https://hydrogen.element.io/#/session/123/room/!abc:m.org/lightbox/$abc +// and the hash is handling the `#/session/123/room/!abc:m.org/lightbox/$abc` +// part class InMemoryHistory extends BaseObservableValue { - #hash = '#/session/123/room/!abc:matrix.org'; + #hash = '#/session/123/room/!abc:m.org'; + + constructor(roomId) { + super(); + + assert(roomId); + + // Since we're viewing an archive of a room, let's mimic the URL of the room + // view + this.#hash = `#/session/123/room/${roomId}`; + } get() { return this.#hash; From 9dceb4a330c5f6507e50b501dddfe7a56f3c9bc6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 15:39:37 -0500 Subject: [PATCH 03/10] Remove commented out code that isn't as relevant --- shared/lib/in-memory-history.js | 1 - 1 file changed, 1 deletion(-) diff --git a/shared/lib/in-memory-history.js b/shared/lib/in-memory-history.js index 879b3188..d49d8742 100644 --- a/shared/lib/in-memory-history.js +++ b/shared/lib/in-memory-history.js @@ -54,7 +54,6 @@ class InMemoryHistory extends BaseObservableValue { } getLastUrl() { - //throw new Error('Not implemented in InMemoryHistory'); return this.#hash; } } From f153fcb7f087097e166e7272f154cd298a343fb5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 17:16:02 -0500 Subject: [PATCH 04/10] WIP: Make the lightbox open, not working yet --- shared/ArchiveView.js | 12 ++++++- shared/hydrogen-vm-render-script.js | 49 +++++++++++++++++++++++------ shared/lib/in-memory-history.js | 20 ++++++++++++ 3 files changed, 70 insertions(+), 11 deletions(-) diff --git a/shared/ArchiveView.js b/shared/ArchiveView.js index 84adc4b0..343cafaa 100644 --- a/shared/ArchiveView.js +++ b/shared/ArchiveView.js @@ -1,6 +1,12 @@ 'use strict'; -const { TemplateView, RoomView, RightPanelView, viewClassForTile } = require('hydrogen-view-sdk'); +const { + TemplateView, + RoomView, + RightPanelView, + LightboxView, + viewClassForTile, +} = require('hydrogen-view-sdk'); class ArchiveView extends TemplateView { render(t, vm) { @@ -13,6 +19,10 @@ class ArchiveView extends TemplateView { [ t.view(new RoomView(vm.roomViewModel, viewClassForTile)), t.view(new RightPanelView(vm.rightPanelModel)), + t.mapView( + (vm) => vm.lightboxViewModel, + (lightboxViewModel) => (lightboxViewModel ? new LightboxView(lightboxViewModel) : null) + ), ] ); } diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 5083afb7..df2270f8 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -18,6 +18,7 @@ const { // RoomView, RoomViewModel, ViewModel, + setupLightboxNavigation, } = require('hydrogen-view-sdk'); const ArchiveView = require('matrix-public-archive-shared/ArchiveView'); @@ -89,12 +90,15 @@ async function mountHydrogen() { const navigation = createNavigation(); const inMemoryHistory = new InMemoryHistory(roomData.id); + inMemoryHistory.subscribe(() => null); platform.setNavigation(navigation); const urlRouter = createRouter({ navigation: navigation, //history: platform.history, history: inMemoryHistory, }); + // Make it listen to changes from the history instance + urlRouter.attach(); // Populate the `Navigation` with segments to work from urlRouter.tryRestoreLastUrl(); @@ -136,10 +140,10 @@ async function mountHydrogen() { // }, // }; - const lightbox = navigation.observe('lightbox'); - lightbox.subscribe((eventId) => { - this._updateLightbox(eventId); - }); + // const lightbox = navigation.observe('lightbox'); + // lightbox.subscribe((eventId) => { + // this._updateLightbox(eventId); + // }); const room = { name: roomData.name, @@ -161,9 +165,13 @@ async function mountHydrogen() { const memberEvent = workingStateEventMap[event.user_id]; return makeEventEntryFromEventJson(event, memberEvent); }); - //console.log('eventEntries', eventEntries); console.log('eventEntries', eventEntries.length); + // Map of `event_id` to `EventEntry` + const eventEntriesByEventId = eventEntries.reduce((currentMap, eventEntry) => { + currentMap[eventEntry.id] = eventEntry; + }, {}); + // We have to use `timeline._setupEntries([])` because it sets // `this._allEntries` in `Timeline` and we don't want to use `timeline.load()` // to request remote things. @@ -282,9 +290,9 @@ async function mountHydrogen() { } const fromDate = new Date(fromTimestamp); - const archiveViewModel = { - roomViewModel, - rightPanelModel: { + class ArchiveViewModel extends ViewModel { + roomViewModel = roomViewModel; + rightPanelModel = { activeViewModel: { type: 'custom', customView: RightPanelContentView, @@ -295,8 +303,29 @@ async function mountHydrogen() { calendarDate: fromDate, }), }, - }, - }; + }; + + lightboxViewModel = null; + + constructor(options) { + super(options); + + this.#setupNavigation(); + } + + #setupNavigation() { + setupLightboxNavigation(this, 'lightboxViewModel'); + } + + _roomFromNavigation() { + return room; + } + } + + const archiveViewModel = new ArchiveViewModel({ + navigation: navigation, + urlCreator: urlRouter, + }); const view = new ArchiveView(archiveViewModel); diff --git a/shared/lib/in-memory-history.js b/shared/lib/in-memory-history.js index d49d8742..c4f357af 100644 --- a/shared/lib/in-memory-history.js +++ b/shared/lib/in-memory-history.js @@ -23,6 +23,14 @@ class InMemoryHistory extends BaseObservableValue { this.#hash = `#/session/123/room/${roomId}`; } + handleEvent(event) { + console.log('handleEvent event', event); + if (event.type === 'hashchange') { + this.#hash = document.location.hash; + this.emit(this.get()); + } + } + get() { return this.#hash; } @@ -30,15 +38,18 @@ class InMemoryHistory extends BaseObservableValue { /** does not emit */ replaceUrlSilently(url) { this.#hash = url; + this.emit(this.get()); } /** does not emit */ pushUrlSilently(url) { this.#hash = url; + this.emit(this.get()); } pushUrl(url) { this.#hash = url; + this.emit(this.get()); } urlAsPath(url) { @@ -53,6 +64,15 @@ class InMemoryHistory extends BaseObservableValue { return `#${path}`; } + onSubscribeFirst() { + console.log('InMemoryHistory: onSubscribeFirst'); + window.addEventListener('hashchange', this); + } + + onUnsubscribeLast() { + window.removeEventListener('hashchange', this); + } + getLastUrl() { return this.#hash; } From d766028423c4c1f4f0ef618b22ff60ebb557fd99 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 17:55:28 -0500 Subject: [PATCH 05/10] Working lightbox pops up and closes --- shared/hydrogen-vm-render-script.js | 14 +++++++------- shared/lib/in-memory-history.js | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index df2270f8..27700bbc 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -170,6 +170,7 @@ async function mountHydrogen() { // Map of `event_id` to `EventEntry` const eventEntriesByEventId = eventEntries.reduce((currentMap, eventEntry) => { currentMap[eventEntry.id] = eventEntry; + return currentMap; }, {}); // We have to use `timeline._setupEntries([])` because it sets @@ -305,8 +306,6 @@ async function mountHydrogen() { }, }; - lightboxViewModel = null; - constructor(options) { super(options); @@ -314,11 +313,12 @@ async function mountHydrogen() { } #setupNavigation() { - setupLightboxNavigation(this, 'lightboxViewModel'); - } - - _roomFromNavigation() { - return room; + setupLightboxNavigation(this, 'lightboxViewModel', (eventId) => { + return { + room, + eventEntry: eventEntriesByEventId[eventId], + }; + }); } } diff --git a/shared/lib/in-memory-history.js b/shared/lib/in-memory-history.js index c4f357af..1bdf577a 100644 --- a/shared/lib/in-memory-history.js +++ b/shared/lib/in-memory-history.js @@ -19,8 +19,8 @@ class InMemoryHistory extends BaseObservableValue { assert(roomId); // Since we're viewing an archive of a room, let's mimic the URL of the room - // view - this.#hash = `#/session/123/room/${roomId}`; + // view unless a hash already exists + this.#hash = document?.location?.hash || `#/session/123/room/${roomId}`; } handleEvent(event) { From eb44fe1b1d62042327ef07b76ad1e8a58a31bff6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 19:37:56 -0500 Subject: [PATCH 06/10] URL hashes relative to the room of the archive This way we can get rid of the bogus session and the duplicated roomId in the URL. Example result: `http://localhost:3050/!HBehERstyQBxyJDLfR:my.synapse.server/date/2022/02/24#/lightbox/$17cgP6YBP9ny9xuU1vBmpOYFhRG4zpOe9SOgWi2Wxsk` --- Before: ``` ``` After: ``` ``` --- shared/hydrogen-vm-render-script.js | 20 +++++++++------ shared/lib/archive-history.js | 38 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 shared/lib/archive-history.js diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 27700bbc..9bbaf35f 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -25,6 +25,7 @@ const ArchiveView = require('matrix-public-archive-shared/ArchiveView'); const RightPanelContentView = require('matrix-public-archive-shared/RightPanelContentView'); const MatrixPublicArchiveURLCreator = require('matrix-public-archive-shared/lib/url-creator'); const InMemoryHistory = require('matrix-public-archive-shared/lib/in-memory-history'); +const ArchiveHistory = require('matrix-public-archive-shared/lib/archive-history'); const fromTimestamp = window.matrixPublicArchiveContext.fromTimestamp; assert(fromTimestamp); @@ -89,18 +90,23 @@ async function mountHydrogen() { }); const navigation = createNavigation(); - const inMemoryHistory = new InMemoryHistory(roomData.id); - inMemoryHistory.subscribe(() => null); + // const inMemoryHistory = new InMemoryHistory(roomData.id); + // // Make it listen to hashchange + // inMemoryHistory.subscribe(() => null); + const archiveHistory = new ArchiveHistory(roomData.id); + // // Make it listen to hashchange + // archiveHistory.subscribe(() => null); platform.setNavigation(navigation); const urlRouter = createRouter({ navigation: navigation, - //history: platform.history, - history: inMemoryHistory, + // history: platform.history, + // history: inMemoryHistory, + history: archiveHistory, }); - // Make it listen to changes from the history instance + // Make it listen to changes from the history instance. And populate the + // `Navigation` with path segments to work from so `href`'s rendered on the + // page don't say `undefined`. urlRouter.attach(); - // Populate the `Navigation` with segments to work from - urlRouter.tryRestoreLastUrl(); // We use the timeline to setup the relations between entries const timeline = new Timeline({ diff --git a/shared/lib/archive-history.js b/shared/lib/archive-history.js new file mode 100644 index 00000000..44f4a851 --- /dev/null +++ b/shared/lib/archive-history.js @@ -0,0 +1,38 @@ +'use strict'; + +const { History } = require('hydrogen-view-sdk'); +const assert = require('./assert'); + +class ArchiveHistory extends History { + constructor(roomId) { + super(); + + assert(roomId); + this._baseHash = `#/session/123/room/${roomId}`; + } + + get() { + const hash = super.get()?.replace(/^#/, '') ?? ''; + return this._baseHash + hash; + } + + replaceUrlSilently(url) { + // We don't need to do this when server-side rendering in Node.js + // because we the #hash is not available to servers. + if (window.history) { + super.replaceUrlSilently(url); + } + } + + pathAsUrl(path) { + // FIXME: When closing the modal it reloads the page instead of SPA because + // this makes `href=""` for the close button. + const leftoverPath = super.pathAsUrl(path).replace(this._baseHash, ''); + if (leftoverPath.length) { + return `#${leftoverPath}`; + } + return leftoverPath; + } +} + +module.exports = ArchiveHistory; From 0763c7829431c978928e6b70d3e5be1e0f075973 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 20:18:38 -0500 Subject: [PATCH 07/10] Fix blank hrefs reloading page --- server/routes/install-routes.js | 6 +++++- server/start-dev.js | 31 +++++++++++++++++++---------- shared/hydrogen-vm-render-script.js | 29 +++++++++++++++++++++++++++ shared/lib/archive-history.js | 6 ++++-- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/server/routes/install-routes.js b/server/routes/install-routes.js index 3f72711c..cef798b2 100644 --- a/server/routes/install-routes.js +++ b/server/routes/install-routes.js @@ -71,7 +71,11 @@ function installRoutes(app) { app.get('/hydrogen-styles.css', async function (req, res) { res.set('Content-Type', 'text/css'); - res.sendFile(require.resolve('hydrogen-view-sdk/style.css')); + // We have to disable no-missing-require lint because it doesn't take into + // account `package.json`. `exports`, see + // https://github.com/mysticatea/eslint-plugin-node/issues/255 + // eslint-disable-next-line node/no-missing-require + res.sendFile(require.resolve('hydrogen-view-sdk/assets/theme-element-light.css')); }); // Our own archive app styles diff --git a/server/start-dev.js b/server/start-dev.js index 982bd2a1..e90755d7 100644 --- a/server/start-dev.js +++ b/server/start-dev.js @@ -1,5 +1,7 @@ 'use strict'; +console.log('start-dev process.env.NODE_ENV', process.env.NODE_ENV); + const path = require('path'); const nodemon = require('nodemon'); const { build } = require('vite'); @@ -7,7 +9,15 @@ const mergeOptions = require('merge-options'); const viteConfig = require('../vite.config'); -console.log('start-dev process.env.NODE_ENV', process.env.NODE_ENV); +// Build the client-side JavaScript bundle when we see any changes +build( + mergeOptions(viteConfig, { + build: { + // Rebuild when we see changes + watch: true, + }, + }) +); // Listen for any changes to files and restart the Node.js server process // @@ -30,16 +40,15 @@ nodemon }) .on('restart', function (files) { console.log('App restarted due to: ', files); - }); - -// Build the client-side JavaScript bundle when we see any changes -build( - mergeOptions(viteConfig, { - build: { - // Rebuild when we see changes - watch: true, - }, }) -); + .on('crash', function () { + console.log('Nodemon: script crashed for some reason'); + }) + // .on('watching', (file) => { + // console.log('watching'); + // }) + .on('log', function (data) { + console.log(`Nodemon logs: ${data.type}: ${data.message}`); + }); console.log('start-dev2 process.env.NODE_ENV', process.env.NODE_ENV); diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 9bbaf35f..192f6505 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -76,6 +76,33 @@ function makeEventEntryFromEventJson(eventJson, memberEvent) { return eventEntry; } +// For any `` (anchor with a blank href), instead of reloading +// the page just remove the hash. +// +// For example, when closing the lightbox via the close "x" icon, it would +// reload the page instead of SPA because `href=""` will cause a page +// navigation if we didn't have this code. +function supressBlankAnchorsReloadingThePage() { + document.body.addEventListener('click', { + handleEvent(e) { + // For any `` (anchor with a blank href), instead of reloading + // the page just remove the hash. + if ( + e.type === 'click' && + e.target.tagName?.toLowerCase() === 'a' && + e.target?.getAttribute('href') === '' + ) { + // Cause a `hashchange` event to be fired + document.location.hash = ''; + // Cleanup the leftover `#` left on the URL + window.history.replaceState(null, null, window.location.pathname); + // Prevent the page navigation (reload) + e.preventDefault(); + } + }, + }); +} + // eslint-disable-next-line max-statements async function mountHydrogen() { const app = document.querySelector('#app'); @@ -339,6 +366,8 @@ async function mountHydrogen() { app.replaceChildren(view.mount()); addSupportClasses(); + + supressBlankAnchorsReloadingThePage(); } // N.B.: When we run this in a `vm`, it will return the last statement. It's diff --git a/shared/lib/archive-history.js b/shared/lib/archive-history.js index 44f4a851..80215c8a 100644 --- a/shared/lib/archive-history.js +++ b/shared/lib/archive-history.js @@ -25,9 +25,11 @@ class ArchiveHistory extends History { } pathAsUrl(path) { - // FIXME: When closing the modal it reloads the page instead of SPA because - // this makes `href=""` for the close button. const leftoverPath = super.pathAsUrl(path).replace(this._baseHash, ''); + // Only add back the hash when there is hash content beyond the base so we + // don't end up with an extraneous `#` on the end of the URL. This will end + // up creating some `` (anchors with a blank href) but we have + // some code to clean this up, see `supressBlankAnchorsReloadingThePage`. if (leftoverPath.length) { return `#${leftoverPath}`; } From fc731bc6dc32be8457b55b6430f9c80babbe12d8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 20:32:50 -0500 Subject: [PATCH 08/10] Some cleanup --- shared/hydrogen-vm-render-script.js | 12 ++--- shared/lib/archive-history.js | 8 ++- shared/lib/in-memory-history.js | 81 ----------------------------- 3 files changed, 10 insertions(+), 91 deletions(-) delete mode 100644 shared/lib/in-memory-history.js diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 192f6505..034f0a5a 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -24,7 +24,6 @@ const { const ArchiveView = require('matrix-public-archive-shared/ArchiveView'); const RightPanelContentView = require('matrix-public-archive-shared/RightPanelContentView'); const MatrixPublicArchiveURLCreator = require('matrix-public-archive-shared/lib/url-creator'); -const InMemoryHistory = require('matrix-public-archive-shared/lib/in-memory-history'); const ArchiveHistory = require('matrix-public-archive-shared/lib/archive-history'); const fromTimestamp = window.matrixPublicArchiveContext.fromTimestamp; @@ -117,17 +116,14 @@ async function mountHydrogen() { }); const navigation = createNavigation(); - // const inMemoryHistory = new InMemoryHistory(roomData.id); - // // Make it listen to hashchange - // inMemoryHistory.subscribe(() => null); const archiveHistory = new ArchiveHistory(roomData.id); - // // Make it listen to hashchange - // archiveHistory.subscribe(() => null); platform.setNavigation(navigation); const urlRouter = createRouter({ navigation: navigation, - // history: platform.history, - // history: inMemoryHistory, + // We use our own history because we want the hash to be relative to the + // room and not include the session/room. + // + // Normally, people use `history: platform.history,` history: archiveHistory, }); // Make it listen to changes from the history instance. And populate the diff --git a/shared/lib/archive-history.js b/shared/lib/archive-history.js index 80215c8a..4f88a596 100644 --- a/shared/lib/archive-history.js +++ b/shared/lib/archive-history.js @@ -3,6 +3,8 @@ const { History } = require('hydrogen-view-sdk'); const assert = require('./assert'); +// Mock a full hash whenever someone asks via `history.get()` but make URL's +// relative to the room (remove session and room) from the hash. class ArchiveHistory extends History { constructor(roomId) { super(); @@ -17,8 +19,10 @@ class ArchiveHistory extends History { } replaceUrlSilently(url) { - // We don't need to do this when server-side rendering in Node.js - // because we the #hash is not available to servers. + // We don't need to do this when server-side rendering in Node.js because + // the #hash is not available to servers. This will be called as a + // downstream call of `urlRouter.attach()` which we do when bootstraping + // everything. if (window.history) { super.replaceUrlSilently(url); } diff --git a/shared/lib/in-memory-history.js b/shared/lib/in-memory-history.js deleted file mode 100644 index 1bdf577a..00000000 --- a/shared/lib/in-memory-history.js +++ /dev/null @@ -1,81 +0,0 @@ -'use strict'; - -const { BaseObservableValue } = require('hydrogen-view-sdk'); -const assert = require('./assert'); - -// A Hydrogen History implementation that doesn't touch the URL. This way -// Hydrogen can still route internally while keeping our external facing URL the -// same. -// -// ex. https://hydrogen.element.io/#/session/123/room/!abc:m.org/lightbox/$abc -// and the hash is handling the `#/session/123/room/!abc:m.org/lightbox/$abc` -// part -class InMemoryHistory extends BaseObservableValue { - #hash = '#/session/123/room/!abc:m.org'; - - constructor(roomId) { - super(); - - assert(roomId); - - // Since we're viewing an archive of a room, let's mimic the URL of the room - // view unless a hash already exists - this.#hash = document?.location?.hash || `#/session/123/room/${roomId}`; - } - - handleEvent(event) { - console.log('handleEvent event', event); - if (event.type === 'hashchange') { - this.#hash = document.location.hash; - this.emit(this.get()); - } - } - - get() { - return this.#hash; - } - - /** does not emit */ - replaceUrlSilently(url) { - this.#hash = url; - this.emit(this.get()); - } - - /** does not emit */ - pushUrlSilently(url) { - this.#hash = url; - this.emit(this.get()); - } - - pushUrl(url) { - this.#hash = url; - this.emit(this.get()); - } - - urlAsPath(url) { - if (url.startsWith('#')) { - return url.substr(1); - } else { - return url; - } - } - - pathAsUrl(path) { - return `#${path}`; - } - - onSubscribeFirst() { - console.log('InMemoryHistory: onSubscribeFirst'); - window.addEventListener('hashchange', this); - } - - onUnsubscribeLast() { - window.removeEventListener('hashchange', this); - } - - getLastUrl() { - return this.#hash; - } -} - -module.exports = InMemoryHistory; From a9496203482ac1d6c46b7994a0eade2bbc296095 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 22:34:29 -0500 Subject: [PATCH 09/10] Lightbox escape keyboard shortcut also works --- shared/hydrogen-vm-render-script.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 034f0a5a..595071d8 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -82,7 +82,13 @@ function makeEventEntryFromEventJson(eventJson, memberEvent) { // reload the page instead of SPA because `href=""` will cause a page // navigation if we didn't have this code. function supressBlankAnchorsReloadingThePage() { - document.body.addEventListener('click', { + const eventHandler = { + clearHash() { + // Cause a `hashchange` event to be fired + document.location.hash = ''; + // Cleanup the leftover `#` left on the URL + window.history.replaceState(null, null, window.location.pathname); + }, handleEvent(e) { // For any `` (anchor with a blank href), instead of reloading // the page just remove the hash. @@ -91,15 +97,19 @@ function supressBlankAnchorsReloadingThePage() { e.target.tagName?.toLowerCase() === 'a' && e.target?.getAttribute('href') === '' ) { - // Cause a `hashchange` event to be fired - document.location.hash = ''; - // Cleanup the leftover `#` left on the URL - window.history.replaceState(null, null, window.location.pathname); + this.clearHash(); // Prevent the page navigation (reload) e.preventDefault(); } + // Also cleanup whenever the hash is emptied out (like when pressing escape in the lightbox) + else if (e.type === 'hashchange' && document.location.hash === '') { + this.clearHash(); + } }, - }); + }; + + document.addEventListener('click', eventHandler); + window.addEventListener('hashchange', eventHandler); } // eslint-disable-next-line max-statements @@ -116,8 +126,9 @@ async function mountHydrogen() { }); const navigation = createNavigation(); - const archiveHistory = new ArchiveHistory(roomData.id); platform.setNavigation(navigation); + + const archiveHistory = new ArchiveHistory(roomData.id); const urlRouter = createRouter({ navigation: navigation, // We use our own history because we want the hash to be relative to the @@ -354,6 +365,7 @@ async function mountHydrogen() { const archiveViewModel = new ArchiveViewModel({ navigation: navigation, urlCreator: urlRouter, + history: archiveHistory, }); const view = new ArchiveView(archiveViewModel); From 3237399c409d7113652fed4f50c10d401c8064bb Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Jun 2022 22:50:27 -0500 Subject: [PATCH 10/10] Some cleanup --- shared/hydrogen-vm-render-script.js | 70 +++++------------------------ shared/lib/archive-history.js | 10 ++++- 2 files changed, 19 insertions(+), 61 deletions(-) diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 595071d8..33e5b833 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -14,8 +14,6 @@ const { encodeKey, encodeEventIdKey, Timeline, - // TimelineView, - // RoomView, RoomViewModel, ViewModel, setupLightboxNavigation, @@ -75,12 +73,14 @@ function makeEventEntryFromEventJson(eventJson, memberEvent) { return eventEntry; } -// For any `` (anchor with a blank href), instead of reloading -// the page just remove the hash. +// For any `` (anchor with a blank href), instead of reloading the +// page just remove the hash. Also cleanup whenever the hash changes for +// whatever reason. // -// For example, when closing the lightbox via the close "x" icon, it would -// reload the page instead of SPA because `href=""` will cause a page -// navigation if we didn't have this code. +// For example, when closing the lightbox by clicking the close "x" icon, it +// would reload the page instead of SPA because `href=""` will cause a page +// navigation if we didn't have this code. Also cleanup whenever the hash is +// emptied out (like when pressing escape in the lightbox). function supressBlankAnchorsReloadingThePage() { const eventHandler = { clearHash() { @@ -145,46 +145,15 @@ async function mountHydrogen() { // We use the timeline to setup the relations between entries const timeline = new Timeline({ roomId: roomData.id, - //storage: this._storage, fragmentIdComparer: fragmentIdComparer, clock: platform.clock, logger: platform.logger, - //hsApi: this._hsApi }); const mediaRepository = new MediaRepository({ homeserver: config.matrixServerUrl, }); - // const urlRouter = { - // urlUntilSegment: () => { - // return 'todo'; - // }, - // urlForSegments: (segments) => { - // const isLightBox = segments.find((segment) => { - // return segment.type === 'lightbox'; - // console.log('segment', segment); - // }); - - // if (isLightBox) { - // return '#'; - // } - - // return 'todo'; - // }, - // }; - - // const navigation = { - // segment: (type, value) => { - // return new Segment(type, value); - // }, - // }; - - // const lightbox = navigation.observe('lightbox'); - // lightbox.subscribe((eventId) => { - // this._updateLightbox(eventId); - // }); - const room = { name: roomData.name, id: roomData.id, @@ -252,23 +221,6 @@ async function mountHydrogen() { tiles, }; - // const view = new TimelineView(timelineViewModel); - - // const roomViewModel = { - // kind: 'room', - // timelineViewModel, - // composerViewModel: { - // kind: 'none', - // }, - // i18n: RoomViewModel.prototype.i18n, - - // id: roomData.id, - // name: roomData.name, - // avatarUrl(size) { - // return getAvatarHttpUrl(roomData.avatarUrl, size, platform, mediaRepository); - // }, - // }; - const roomViewModel = new RoomViewModel({ room, ownUserId: 'xxx', @@ -277,6 +229,7 @@ async function mountHydrogen() { navigation, }); + // FIXME: We shouldn't have to dive into the internal fields to make this work roomViewModel._timelineVM = timelineViewModel; roomViewModel._composerVM = { kind: 'none', @@ -370,7 +323,6 @@ async function mountHydrogen() { const view = new ArchiveView(archiveViewModel); - //console.log('view.mount()', view.mount()); app.replaceChildren(view.mount()); addSupportClasses(); @@ -378,7 +330,7 @@ async function mountHydrogen() { supressBlankAnchorsReloadingThePage(); } -// N.B.: When we run this in a `vm`, it will return the last statement. It's -// important to leave this as the last statement so we can await the promise it -// returns and signal that all of the async tasks completed. +// N.B.: When we run this in a virtual machine (`vm`), it will return the last +// statement. It's important to leave this as the last statement so we can await +// the promise it returns and signal that all of the async tasks completed. mountHydrogen(); diff --git a/shared/lib/archive-history.js b/shared/lib/archive-history.js index 4f88a596..35d0a304 100644 --- a/shared/lib/archive-history.js +++ b/shared/lib/archive-history.js @@ -3,8 +3,9 @@ const { History } = require('hydrogen-view-sdk'); const assert = require('./assert'); -// Mock a full hash whenever someone asks via `history.get()` but make URL's -// relative to the room (remove session and room) from the hash. +// Mock a full hash whenever someone asks via `history.get()` but when +// constructing URL's for use `href` etc, they should relative to the room +// (remove session and room from the hash). class ArchiveHistory extends History { constructor(roomId) { super(); @@ -13,6 +14,8 @@ class ArchiveHistory extends History { this._baseHash = `#/session/123/room/${roomId}`; } + // Even though the page hash is relative to the room, we still expose the full + // hash for Hydrogen to route things internally as expected. get() { const hash = super.get()?.replace(/^#/, '') ?? ''; return this._baseHash + hash; @@ -28,6 +31,9 @@ class ArchiveHistory extends History { } } + // Make the URLs we use in the UI of the app relative to the room: + // Before: #/session/123/room/!HBehERstyQBxyJDLfR:my.synapse.server/lightbox/$17cgP6YBP9ny9xuU1vBmpOYFhRG4zpOe9SOgWi2Wxsk + // After: #/lightbox/$17cgP6YBP9ny9xuU1vBmpOYFhRG4zpOe9SOgWi2Wxsk pathAsUrl(path) { const leftoverPath = super.pathAsUrl(path).replace(this._baseHash, ''); // Only add back the hash when there is hash content beyond the base so we