Skip to content

Commit cc78f10

Browse files
authored
Merge branch 'dev' into server-over-https
2 parents 7eb706b + 36e6523 commit cc78f10

22 files changed

Lines changed: 638 additions & 57 deletions

.agents/context/backend.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
- `py/visdom/server/app.py` — Application class, routes, state management.
99
- `py/visdom/server/handlers/web_handlers.py` — HTTP handlers. Copy app attributes in `initialize()`, use `@check_auth`.
1010
- `py/visdom/server/handlers/socket_handlers.py` — WebSocket handlers (read-only + write-enabled).
11-
- `py/visdom/utils/server_utils.py``check_auth`, `broadcast`, `LazyEnvData`, `serialize_env`.
11+
- `py/visdom/utils/server_utils.py``check_auth`, `broadcast`, `LazyEnvData`. Environment persistence lives in `py/visdom/data_model/json_store.py` (`JSONStore`).
1212
- `py/visdom/server/build.py``download_scripts()`: fetches CDN dependencies.
1313

1414
## Coding Rules

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ cypress/screenshots
22
cypress/screenshots_init
33
cypress/downloads
44
cypress/fixtures
5+
playwright/screenshots
6+
playwright/screenshots_init
7+
playwright/screenshots_diff
58
node_modules
69
build
710
th/CMakeLists.txt

REFACTORING.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ This is the primary TODO from PR #675: *"move the logic that actually parses env
7979
| File | Purpose |
8080
|------|---------|
8181
| `environment.py` | `Environment` class wrapping `{"jsons": {}, "reload": {}}` dict. Methods: `get_window()`, `set_window()`, `remove_window()`, `list_windows()`, `get_reload()` |
82-
| `state_manager.py` | `StateManager` class wrapping the top-level `state` dict. Methods: `get_env()`, `create_env()`, `delete_env()`, `list_envs()`, `fork_env()`, `serialize()`, `serialize_all()`. Absorbs functions from `server_utils.py` (`serialize_env`, `serialize_all`, `load_env`, `gather_envs`, `compare_envs`) |
82+
| `state_manager.py` | `StateManager` class wrapping the top-level `state` dict. Methods: `get_env()`, `create_env()`, `delete_env()`, `list_envs()`, `fork_env()`, `serialize()`, `serialize_all()`. Absorbs functions from `server_utils.py` (`load_env`, `gather_envs`, `compare_envs`). Environment file serialization now lives in `data_model/json_store.py` (`JSONStore`) via the `DataStore` abstraction |
8383
| `window.py` | Typed window structures (dataclasses/TypedDicts) replacing raw dicts built in `server_utils.py:window()` (lines 202-258) |
8484

8585
### 3b. Refactor handlers to use data model
@@ -109,12 +109,12 @@ py/visdom/server/
109109
**Effort: Medium-Large | Risk: Medium | PRs: 2 | Dependencies: Phase 3**
110110

111111
### 4a. Wrap blocking file I/O with `run_in_executor`
112-
Currently only **ONE** place uses async I/O (`socket_handlers.py:123` for `serialize_all`). All others block the Tornado event loop:
112+
Currently only **ONE** place uses async I/O (`socket_handlers.py:123` for `storage.save_all`). All others block the Tornado event loop:
113113

114114
| File | Lines | Blocking Operation |
115115
|------|-------|-------------------|
116116
| `server_utils.py` | 72 | Cookie file write |
117-
| `server_utils.py` | 121-155 | `serialize_env()` — JSON file write |
117+
| `data_model/json_store.py` | `save_envs()` | `JSONStore` — JSON file write |
118118
| `server_utils.py` | 273-303 | `compare_envs()` — env file read |
119119
| `app.py` | 141 | Layout save (file write) |
120120
| `app.py` | 155 | Layout load (file read) |

js/api/ApiProvider.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,18 @@ const ApiProvider = ({ children }) => {
336336
});
337337
};
338338

339+
const sendCommentUpdate = (envID, win, comment) => {
340+
if (win === null || sessionInfo.readonly) {
341+
return;
342+
}
343+
sendSocketMessage({
344+
cmd: 'update_comment',
345+
eid: envID,
346+
win: win,
347+
data: comment,
348+
});
349+
};
350+
339351
// Save layout lists to the server
340352
const sendLayoutsSave = (layoutLists) => {
341353
// pushes layouts to the server
@@ -387,6 +399,7 @@ const ApiProvider = ({ children }) => {
387399
value={{
388400
apiHandlers,
389401
connected,
402+
sendCommentUpdate,
390403
sendEmbeddingPop,
391404
sendEnvDelete,
392405
sendEnvQuery,

js/panes/Pane.js

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,69 @@
77
*
88
*/
99

10-
import React, { forwardRef, useRef, useState } from 'react';
10+
import React, {
11+
forwardRef,
12+
useContext,
13+
useEffect,
14+
useRef,
15+
useState,
16+
} from 'react';
1117

18+
import ApiContext from '../api/ApiContext';
1219
import PropertyItem from './PropertyItem';
1320
var classNames = require('classnames');
1421

22+
const COMMENT_SAVE_DEBOUNCE_MS = 500;
23+
1524
var Pane = forwardRef((props, ref) => {
1625
const { id, title, content, children, widgets, enablePropertyList } = props;
1726
var { barwidgets } = props;
1827
barwidgets = barwidgets || [];
28+
const commentEnabled = !props.commentsDisabled;
1929

2030
// state varibles
2131
// --------------
2232
const [propertyListShown, setPropertyListShown] = useState(false);
2333
const barRef = useRef();
2434

35+
const { sendCommentUpdate, sessionInfo } = useContext(ApiContext);
36+
const [commentOpen, setCommentOpen] = useState(false);
37+
const [commentText, setCommentText] = useState(props.comment || '');
38+
const commentSaveTimeout = useRef(null);
39+
const isEditingComment = useRef(false);
40+
const latestCommentTextRef = useRef(commentText);
41+
42+
useEffect(() => {
43+
if (!isEditingComment.current) {
44+
setCommentText(props.comment || '');
45+
latestCommentTextRef.current = props.comment || '';
46+
}
47+
}, [props.comment]);
48+
49+
useEffect(() => {
50+
return () => {
51+
if (commentSaveTimeout.current) {
52+
flushCommentSave(latestCommentTextRef.current);
53+
}
54+
};
55+
}, []);
56+
57+
const flushCommentSave = (value) => {
58+
clearTimeout(commentSaveTimeout.current);
59+
commentSaveTimeout.current = null;
60+
sendCommentUpdate(props.envID, id, value);
61+
};
62+
63+
const handleCommentChange = (ev) => {
64+
const value = ev.target.value;
65+
setCommentText(value);
66+
latestCommentTextRef.current = value;
67+
clearTimeout(commentSaveTimeout.current);
68+
commentSaveTimeout.current = setTimeout(() => {
69+
flushCommentSave(value);
70+
}, COMMENT_SAVE_DEBOUNCE_MS);
71+
};
72+
2573
// public events
2674
// -------------
2775
const handleOnFocus = props.handleOnFocus || (() => props.onFocus(id));
@@ -37,6 +85,11 @@ var Pane = forwardRef((props, ref) => {
3785
let windowClassNames = classNames({ window: true, focus: props.isFocused });
3886
let barClassNames = classNames({ bar: true, focus: props.isFocused });
3987

88+
let contentClassNames = classNames({
89+
content: true,
90+
'content-with-comment': commentEnabled && commentOpen,
91+
});
92+
4093
// add property list button to barwidgets
4194
if (
4295
enablePropertyList &&
@@ -129,10 +182,38 @@ var Pane = forwardRef((props, ref) => {
129182
{' '}
130183
⟲{' '}
131184
</button>
185+
<button
186+
title="comment"
187+
onClick={() => setCommentOpen(!commentOpen)}
188+
className={commentOpen ? 'pull-right active' : 'pull-right'}
189+
hidden={!commentEnabled}
190+
>
191+
<span className="glyphicon glyphicon-comment" />
192+
</button>
132193
{barwidgets}
133194
<div className="pull-right">{title}</div>
134195
</div>
135-
<div className="content">{children}</div>
196+
<div className={contentClassNames}>
197+
{children}
198+
{commentEnabled && commentOpen && (
199+
<div className="comment-panel">
200+
<textarea
201+
className="comment-textarea"
202+
placeholder="Add a note for this pane..."
203+
value={commentText}
204+
onFocus={() => {
205+
isEditingComment.current = true;
206+
}}
207+
onBlur={() => {
208+
isEditingComment.current = false;
209+
flushCommentSave(commentText);
210+
}}
211+
onChange={handleCommentChange}
212+
readOnly={!!sessionInfo?.readonly}
213+
/>
214+
</div>
215+
)}
216+
</div>
136217
<div className="widgets">{widgets}</div>
137218
{propertyListOverlay}
138219
</div>
@@ -147,6 +228,7 @@ Pane = React.memo(Pane, (props, nextProps) => {
147228
else if (props.h !== nextProps.h || props.w !== nextProps.w) return false;
148229
else if (props.children !== nextProps.children) return false;
149230
else if (props.isFocused !== nextProps.isFocused) return false;
231+
else if (props.comment !== nextProps.comment) return false;
150232
return true;
151233
});
152234

js/panes/PlotPane.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ var PlotPane = (props) => {
183183
smooth_d.showlegend = false;
184184

185185
// turn off smoothing for smoothvalue of 3 or too small arrays
186-
if (windowSize < 5 || smooth_d.x.length <= 5) {
186+
if (windowSize < 5 || !smooth_d.x || smooth_d.x.length <= 5) {
187187
d.opacity = 1.0;
188188

189189
return smooth_d;

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"test:visual": "cypress run --spec './cypress/integration/screenshots.js'",
3939
"test": "cypress run --config excludeSpecPattern=**/*.init.js",
4040
"test:pw": "playwright test --config=playwright.config.js",
41+
"test:pw:init": "playwright test --config=playwright.init.config.js",
4142
"lint": "eslint js/.",
4243
"lint:fix": "eslint --fix --ext .js,.jsx js/"
4344
},

playwright.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ module.exports = defineConfig({
2323
retries: process.env.CI ? 2 : 0,
2424
workers: 1,
2525
outputDir: './playwright/test-results',
26+
testIgnore: ['**/*.init.spec.js'],
2627
reporter: [['html', { outputFolder: 'playwright/playwright-report' }]],
2728
use: {
2829
baseURL: 'http://localhost:8098',

playwright.init.config.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* Copyright 2017-present, The Visdom Authors
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*
8+
*/
9+
10+
const { defineConfig } = require('@playwright/test');
11+
const baseConfig = require('./playwright.config');
12+
13+
module.exports = defineConfig({
14+
...baseConfig,
15+
testIgnore: [],
16+
testMatch: '**/*.init.spec.js',
17+
});

playwright/support/helpers.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,45 @@ async function openEnv(page, name) {
121121
await closeEnvDropdown(page);
122122
}
123123

124+
async function waitForPlotRender(page) {
125+
await page
126+
.locator('.content')
127+
.first()
128+
.waitFor({ state: 'visible', timeout: 20000 });
129+
await page.waitForTimeout(800);
130+
}
131+
132+
async function waitForMathJax(page) {
133+
await page.waitForTimeout(2000);
134+
await page.evaluate(async () => {
135+
if (window.MathJax && window.MathJax.Hub) {
136+
await new Promise((resolve) => {
137+
window.MathJax.Hub.Queue(() => resolve());
138+
});
139+
}
140+
141+
if (document.fonts && document.fonts.ready) {
142+
await document.fonts.ready;
143+
}
144+
});
145+
await page.waitForTimeout(2000);
146+
}
147+
148+
async function screenshotContent(page, screenshotPath) {
149+
fs.mkdirSync(path.dirname(screenshotPath), { recursive: true });
150+
151+
const content = page.locator('.content').first();
152+
await content.waitFor({ state: 'visible', timeout: 20000 });
153+
await content.screenshot({ path: screenshotPath });
154+
}
155+
124156
module.exports = {
125157
runDemo,
126158
closeEnvs,
127159
expandAllEnvGroups,
128160
closeEnvDropdown,
129161
openEnv,
162+
waitForPlotRender,
163+
waitForMathJax,
164+
screenshotContent,
130165
};

0 commit comments

Comments
 (0)