ntfy icon indicating copy to clipboard operation
ntfy copied to clipboard

Use relative paths in WebUI

Open Curid opened this issue 2 years ago • 16 comments

Closes #256

Curid avatar May 16 '22 17:05 Curid

It's a bit buggy at the moment. I tried to mark it as a draft.

The only modified react-script is ./web/react-scripts/config/webpack.config.js. There's probably a better way to change it.

Curid avatar May 16 '22 17:05 Curid

Cool thank you! The root = window.location.pathname is a genius idea!

Few things:

  1. Is the ./ really necessary? So can't I just do <a href="app"> instead of <a href="./app">?
  2. You committed react-scripts, which should not be in there. Can you remove that?
  3. Be sure to try attachments too, and sounds and the web-root = home setting. I vaguely remember having issues with that.

binwiederhier avatar May 16 '22 17:05 binwiederhier

Is the ./ really necessary? So can't I just do <a href="app"> instead of <a href="./app">?

wtf, now it works without it.

You committed react-scripts, which should not be in there. Can you remove that?

I made a few changes that don't seem necessary either.

+++ webpack.config.js
@@ -218,9 +218,7 @@ module.exports = function (webpackEnv) {
       pathinfo: isEnvDevelopment,
       // There will be one main bundle, and one file per asynchronous chunk.
       // In development, it does not produce real files.
-      filename: isEnvProduction
-        ? 'static/js/[name].[contenthash:8].js'
-        : isEnvDevelopment && 'static/js/bundle.js',
+      filename: 'static/js/bundle.js',
       // There are also additional JS chunk files if you use code splitting.
       chunkFilename: isEnvProduction
         ? 'static/js/[name].[contenthash:8].chunk.js'
@@ -609,7 +607,7 @@ module.exports = function (webpackEnv) {
         Object.assign(
           {},
           {
-            inject: true,
+            inject: false,
             template: paths.appHtml,
           },
           isEnvProduction

Be sure to try attachments too, and sounds and the web-root = home setting. I vaguely remember having issues with that.

I still have to figure out why some static assets don't work.

window.location.pathname doesn't seem to work reliably if you refresh the page. We may have to add # to the root path and set root to that. localhost/#/settings. Or we could just remove the routing?

Curid avatar May 16 '22 18:05 Curid

window.location.pathname doesn't seem to work reliably if you refresh the page. We may have to add # to the root path and set root to that. localhost/#/settings. Or we could just remove the routing?

I don't want hash routing, and I definitely want to keep the routing. If you can't get it to work, then I'd much rather not support relative paths. I think that's why I gave up after an hour -- the routing....

binwiederhier avatar May 16 '22 18:05 binwiederhier

The routing just worked before. That's why I shared it.

It should be doable to search the path for the root. Maybe save it in local storage and validate it by fetching a test file.

Curid avatar May 16 '22 18:05 Curid

The test file thing sounds horrible :-D I'd much rather prefer a different way. This has to be a solved problem, right?

binwiederhier avatar May 16 '22 18:05 binwiederhier

The root must be identifiable in some way, /#/ or /:/ If we only had a single sub directory depth then we could use the last /

Curid avatar May 16 '22 19:05 Curid

https://ntfy.sh/mytopic must be the topic's address. What if you write something when the server initially loads?! Idk, just brainstorming.

binwiederhier avatar May 16 '22 19:05 binwiederhier

I came up with another way to do it. It's a bit dirty but should be reliable.

Curid avatar May 17 '22 16:05 Curid

Codecov Report

Merging #258 (cf680bd) into main (db613f8) will increase coverage by 0.01%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
+ Coverage   65.53%   65.55%   +0.01%     
==========================================
  Files          32       33       +1     
  Lines        3122     3135      +13     
==========================================
+ Hits         2046     2055       +9     
- Misses        776      778       +2     
- Partials      300      302       +2     
Impacted Files Coverage Δ
cmd/config_loader.go 62.50% <0.00%> (ø)
cmd/app.go 100.00% <0.00%> (+22.72%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update db613f8...cf680bd. Read the comment docs.

codecov-commenter avatar May 17 '22 19:05 codecov-commenter

@binwiederhier I got everything working now. It works surprisingly well.

Curid avatar May 18 '22 16:05 Curid

Just acknowledging that I saw this. I will likely not get to this for a while.

binwiederhier avatar May 18 '22 23:05 binwiederhier

I'm just going to say it like it is, don't be mad, but the design and implementation of this is not right. I think I'll eventually need to make the whole thing work on a subdirectory for the Electron app (#239, #112), but I really don't want to probe or sleep or anything. The app should know where it's served from. This has to be a solved problem.

How's this: When you request /subdir1/mytopic, the server returns the HTML of the topic page contains const rootPath = "/subdir1" somewhere in the rendered HTML. How the server figures out the root path I'm not quite sure. Maybe the proxy server sends X-Root-Path or something, idk.

My point is: you should not guess and you should not sleep on the client side.

binwiederhier avatar Jun 03 '22 13:06 binwiederhier

I'm just going to say it like it is, don't be mad

lol, don't worry I know it's bad.

What if the server injects the full base_url into index.html? The client should be able to calculate the pathname from that.

Curid avatar Jun 03 '22 17:06 Curid

How about this?

Curid avatar Jun 04 '22 16:06 Curid

Sorry for jumping on an old pull request, but I wanted to chime in saying subpath networking in the future would be tremendously helpful.

If I have enough time I might also be able to contribute a bit.

Congrats on the kid!

amcraig avatar Oct 13 '22 08:10 amcraig

Well, this is 15 hours I'll never get back.

Curid avatar Dec 12 '22 20:12 Curid