Skip to content

Commit

Permalink
Slightly imporved pyrepl (#1296)
Browse files Browse the repository at this point in the history
  * removed unnecessary getAttribute
  * removed unnecessary shadow and ShadowDOM in general, as it was never used
  * dropped redundant constructor
  * removed unnecessary usage of the label element
  * fixed redundant always-same buttons IDs
  • Loading branch information
WebReflection committed Mar 24, 2023
1 parent 543a272 commit c8becca
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 39 deletions.
41 changes: 7 additions & 34 deletions pyscriptjs/src/components/pyrepl.ts
Expand Up @@ -6,7 +6,7 @@ import { keymap, Command } from '@codemirror/view';
import { defaultKeymap } from '@codemirror/commands';
import { oneDarkTheme } from '@codemirror/theme-one-dark';

import { getAttribute, ensureUniqueId, htmlDecode } from '../utils';
import { ensureUniqueId, htmlDecode } from '../utils';
import { pyExec } from '../pyexec';
import { getLogger } from '../logger';
import { InterpreterClient } from '../interpreter_client';
Expand All @@ -20,31 +20,20 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {
/* High level structure of py-repl DOM, and the corresponding JS names.
this <py-repl>
shadow #shadow-root
<slot></slot>
boxDiv <div class='py-repl-box'>
editorLabel <label>...</label>
editorDiv <div class="py-repl-editor"></div>
outDiv <div class="py-repl-output"></div>
</div>
</py-repl>
*/
class PyRepl extends HTMLElement {
shadow: ShadowRoot;
outDiv: HTMLElement;
editor: EditorView;
stdout_manager: Stdio | null;
stderr_manager: Stdio | null;

constructor() {
super();
}

connectedCallback() {
ensureUniqueId(this);
this.shadow = this.attachShadow({ mode: 'open' });
const slot = document.createElement('slot');
this.shadow.appendChild(slot);

if (!this.hasAttribute('exec-id')) {
this.setAttribute('exec-id', '0');
Expand Down Expand Up @@ -77,7 +66,7 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {
]),
];

if (getAttribute(this, 'theme') === 'dark') {
if (this.getAttribute('theme') === 'dark') {
extensions.push(oneDarkTheme);
}

Expand All @@ -97,10 +86,8 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {
boxDiv.className = 'py-repl-box';

const editorDiv = this.makeEditorDiv();
const editorLabel = this.makeLabel('Python Script Area', editorDiv);
this.outDiv = this.makeOutDiv();

boxDiv.append(editorLabel);
boxDiv.appendChild(editorDiv);
boxDiv.appendChild(this.outDiv);

Expand All @@ -109,35 +96,21 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {

makeEditorDiv(): HTMLElement {
const editorDiv = document.createElement('div');
editorDiv.id = 'code-editor';
editorDiv.className = 'py-repl-editor';
editorDiv.setAttribute('aria-label', 'Python Script Area');
editorDiv.appendChild(this.editor.dom);

const runButton = this.makeRunButton();
const runLabel = this.makeLabel('Python Script Run Button', runButton);
editorDiv.appendChild(runLabel);
editorDiv.appendChild(runButton);

return editorDiv;
}

makeLabel(text: string, elementFor: HTMLElement): HTMLElement {
ensureUniqueId(elementFor);
const lbl = document.createElement('label');
lbl.innerHTML = text;
lbl.htmlFor = elementFor.id;
// XXX this should be a CSS class
// Styles that we use to hide the labels whilst also keeping it accessible for screen readers
const labelStyle = 'overflow:hidden; display:block; width:1px; height:1px';
lbl.setAttribute('style', labelStyle);
return lbl;
}

makeRunButton(): HTMLElement {
const runButton = document.createElement('button');
runButton.id = 'runButton';
runButton.className = 'absolute py-repl-run-button';
runButton.innerHTML = RUNBUTTON;
runButton.setAttribute('aria-label', 'Python Script Run Button');
runButton.addEventListener('click', this.execute.bind(this) as (e: MouseEvent) => void);
return runButton;
}
Expand All @@ -161,13 +134,13 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {
// execute the python code
app.plugins.beforePyReplExec({ interpreter: interpreter, src: pySrc, outEl: outEl, pyReplTag: this });
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const pyResult = (await pyExec(interpreter, pySrc, outEl)).result;
const { result } = await pyExec(interpreter, pySrc, outEl);
app.plugins.afterPyReplExec({
interpreter: interpreter,
src: pySrc,
outEl: outEl,
pyReplTag: this,
result: pyResult, // eslint-disable-line @typescript-eslint/no-unsafe-assignment
result, // eslint-disable-line @typescript-eslint/no-unsafe-assignment
});

this.autogenerateMaybe();
Expand All @@ -190,7 +163,7 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {

//Attributes to be copied from old REPL to auto-generated REPL
for (const attribute of ['root', 'output-mode', 'output', 'stderr']) {
const attr = getAttribute(this, attribute);
const attr = this.getAttribute(attribute);
if (attr) {
newPyRepl.setAttribute(attribute, attr);
}
Expand Down
5 changes: 2 additions & 3 deletions pyscriptjs/tests/integration/test_py_repl.py
Expand Up @@ -24,9 +24,8 @@ def test_repl_loads(self):
<py-repl></py-repl>
"""
)
py_repl = self.page.query_selector("py-repl")
py_repl = self.page.query_selector("py-repl .py-repl-box")
assert py_repl
assert "Python" in py_repl.inner_text()

def test_execute_preloaded_source(self):
"""
Expand Down Expand Up @@ -69,7 +68,7 @@ def test_execute_on_shift_enter(self):
</py-repl>
"""
)
self.page.wait_for_selector("#runButton")
self.page.wait_for_selector("py-repl .py-repl-run-button")
self.page.keyboard.press("Shift+Enter")
wait_for_render(self.page, "*", "hello world")

Expand Down
4 changes: 2 additions & 2 deletions pyscriptjs/tests/integration/test_zz_examples.py
Expand Up @@ -299,7 +299,7 @@ def test_repl(self):
wait_for_render(self.page, "*", "<py-repl.*?>")

self.page.locator("py-repl").type("display('Hello, World!')")
self.page.locator("#runButton").click()
self.page.locator("py-repl .py-repl-run-button").click()

assert (
self.page.locator("#my-repl-repl-output").text_content() == "Hello, World!"
Expand All @@ -322,7 +322,7 @@ def test_repl2(self):
wait_for_render(self.page, "*", "<py-repl.*?>")
# confirm we can import utils and run one command
self.page.locator("py-repl").type("import utils\ndisplay(utils.now())")
self.page.wait_for_selector("#runButton").click()
self.page.wait_for_selector("py-repl .py-repl-run-button").click()
# Make sure the output is in the page
self.page.wait_for_selector("#my-repl-1")
# utils.now returns current date time
Expand Down

0 comments on commit c8becca

Please sign in to comment.