fix: remove invalid user/style.css reference from index.html#1078
fix: remove invalid user/style.css reference from index.html#1078KushDutta23 wants to merge 1 commit intofossasia:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts stylesheet links in the Visdom frontend index.html, replacing the invalid ../user/style.css static path with another reference to css/style.css so all referenced stylesheets resolve via the static_url helper. Sequence diagram for loading stylesheets via static_url after fixsequenceDiagram
actor User
participant Browser
participant Server
User->>Browser: Open Visdom index.html
Browser->>Server: GET /static/index.html
Server-->>Browser: 200 index.html (with css/style.css links)
Browser->>Server: GET static_url(css/style.css)
Server-->>Browser: 200 css/style.css
Browser->>Server: GET static_url(css/network.css)
Server-->>Browser: 200 network.css
Note right of Browser: Apply styles from css/style.css and css/network.css
Note right of User: Sees correctly styled UI without static file errors
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new line still references
css/style.css, resulting in a duplicate stylesheet import instead of actually removing the invalid../user/style.cssreference—consider either deleting the line entirely or pointing it to the intended different stylesheet if one exists. - The indentation of the newly added
<link>tag is off by one space compared to the surrounding lines; aligning it will keep the HTML formatting consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new line still references `css/style.css`, resulting in a duplicate stylesheet import instead of actually removing the invalid `../user/style.css` reference—consider either deleting the line entirely or pointing it to the intended different stylesheet if one exists.
- The indentation of the newly added `<link>` tag is off by one space compared to the surrounding lines; aligning it will keep the HTML formatting consistent.
## Individual Comments
### Comment 1
<location path="py/visdom/static/index.html" line_range="64" />
<code_context>
</script>
<script src={{ static_url("js/main.js") }}></script>
<link rel="stylesheet" href={{ static_url("css/style.css") }}>
- <link rel="stylesheet" href={{ static_url("../user/style.css") }}>
+ <link rel="stylesheet" href={{ static_url("css/style.css") }}>
</code_context>
<issue_to_address>
**issue (bug_risk):** Duplicated reference to css/style.css likely replaces the intended ../user/style.css include.
This now loads `css/style.css` twice and drops the previous `../user/style.css`, which appears to have been intended for user-specific overrides. If those overrides are still needed, please restore the original path or otherwise clarify which distinct stylesheet should be referenced here.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -62,7 +62,7 @@ | |||
| </script> | |||
| <script src={{ static_url("js/main.js") }}></script> | |||
| <link rel="stylesheet" href={{ static_url("css/style.css") }}> | |||
There was a problem hiding this comment.
issue (bug_risk): Duplicated reference to css/style.css likely replaces the intended ../user/style.css include.
This now loads css/style.css twice and drops the previous ../user/style.css, which appears to have been intended for user-specific overrides. If those overrides are still needed, please restore the original path or otherwise clarify which distinct stylesheet should be referenced here.
There was a problem hiding this comment.
Thanks for the feedback!
You're right — the previous change introduced a duplicate reference to css/style.css, which was not the intended fix.
The original ../user/style.css path appears to have been meant for user-specific overrides, but since the path is invalid and no corresponding stylesheet exists in the repository, I have removed the reference entirely instead of duplicating the base stylesheet.
Please let me know if a dedicated user override stylesheet should be added instead.
There was a problem hiding this comment.
Good catch on the missing file – your reasoning makes sense.
Looking at the code and docs, Visdom already supports user‑specific CSS via the config/env paths rather than a ../user/style.css file under static:
py/visdom/server/app.pyandweb_handlers.pyexplicitly handle astyle.cssserved from the user config directory orenv_path.- The README documents this behavior:
By placing a
style.cssin your user config directory, visdom will serve the customized css file along with the default style-file. In addition, it is also possible to create a project-specific file; just place the filestyle.cssin yourenv_path.
So the ../user/style.css reference in index.html was actually redundant and pointed at the wrong place. The correct flow is:
- Always include the base stylesheet:
<link rel="stylesheet" href={{ static_url("css/style.css") }}>
- Let the backend optionally inject an additional
style.cssfrom the user config/env path when present (which the server already supports), instead of hardcoding a second<link>in the HTML.
Given that, your current change (removing the invalid ../user/style.css reference and avoiding a duplicate css/style.css link) is the right fix. No extra user override stylesheet needs to be added under static/.
You might consider adding a short note in the PR description (or commit message) referencing the documented user-CSS mechanism in the README, but code-wise I’m happy with this direction.
Fix: Remove invalid static CSS path
Problem
The frontend attempted to load a stylesheet from
../user/style.css, which does not exist, causing a static file error.Solution
css/style.cssimportResult
Summary by Sourcery
Bug Fixes: