-
-
Notifications
You must be signed in to change notification settings - Fork 221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add basic web UI #1395
add basic web UI #1395
Conversation
(cherry picked from commit d82a237)
specify node and npm versions in package.json
thx! I will be able to review it fully next week (I'm at FOSS4G conference). One thing -- is it possible to make |
P.S. please merge in the latest main branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few minor changes, plus I merged and pushed the latest main
branch. One thing -- for some reason, CI fails for this PR - need to fix that
martin-ui/vite.config.ts
Outdated
// assets can also be the name of a tile source | ||
// so we use _assets to avoid conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at all the requests when viewing localhost:3000/
while setting RUST_LOG=debug
, I see these requests:
[2024-07-09T14:32:23Z INFO actix_web::middleware::logger] 127.0.0.1 "GET / HTTP/1.1" 304 0 "-" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36" 0.000332
[2024-07-09T14:32:23Z INFO actix_web::middleware::logger] 127.0.0.1 "GET /assets/index-0vhmA-Yl.js HTTP/1.1" 304 0 "http://0.0.0.0:3000/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36" 0.000802
[2024-07-09T14:32:23Z INFO actix_web::middleware::logger] 127.0.0.1 "GET /_assets/index.RjhygAlW.css HTTP/1.1" 304 0 "http://0.0.0.0:3000/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36" 0.001011
[2024-07-09T14:32:23Z INFO actix_web::middleware::logger] 127.0.0.1 "GET /_assets/logo.ZTiqb92t.png HTTP/1.1" 304 0 "http://0.0.0.0:3000/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36" 0.000364
So it clearly gets BOTH /assets
and /_assets
-- not good. Moreover, it IS possible to create an _assets
source, so it could conflict. The only way to NOT have any conflicts is to add all assets to /_/assets/*
path -- this way it will never conflict (the _
is a reserved keyword)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that this PR is complete. When I run npm run build && npm run dev
in the martin-ui
sub-directory, the UI runs fine. When I run cargo run -- ./world_cities.mbtiles
, local is unable to find the js file at the root, but is able to find a css file at /assets. I'm not sure what is happening at cargo run ...
that would prevent it from finding the js file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would anything access things at /assets/...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that my latest change meets what you are looking for with js, css and image files all served from _/assets
. Running cargo run -- ./world_cities.mbtiles
now runs the web UI at /
.
If it doesn't, can you outline the directory or output build structure you have in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!! Lets do some basic UI in a next PR - before we do a release
@nyurik Agreed, do you have a list of basic features in mind? I would imagine a map, and maybe a list of checkboxes for the sources available at |
@paigewilliams lets discuss it in #1120 -- there are some ideas there already |
I branched off of #1142 to address the PR feedback from @nyurik .
Let me know if there is a better way to submit these changes for review.