Add main area widget (form in headless mode) with state restoration #12
Conversation
|
|
src/index.ts
Outdated
| ? currentWidget.context.path | ||
| : ''; | ||
|
|
||
| const selectedFilepath = |
There was a problem hiding this comment.
If a filepath field is passed in the queryParameters initially, then this takes precedence. This is important information for restoring the tab later.
Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
nenb
force-pushed
the
headless-mode-v2
branch
from
d7fe955 to
ed7923f
Compare
| "label": "Deploy App" | ||
| } | ||
| ] | ||
| }, | ||
| "title": "jupyterlab-jhub-apps", | ||
| "description": "jupyterlab-jhub-apps custom command settings.", | ||
| "type": "object", | ||
| "properties": {}, | ||
| "properties": { |
There was a problem hiding this comment.
I moved to properties as I was unable to find another way to update the settings values at runtime. This is useful for situations like testing, where I want to test different settings values. I took this approach from https://github.com/jupyterlab/extension-examples/tree/main/settings.
I also noticed that if a user updates their settings, then the application will not notice this. I have now used properties and settings.changed.connect below to handle this.
src/index.ts
Outdated
| * These are read from the settings registry and updated any | ||
| * time a user changes their settings. | ||
| */ | ||
| const runtimeSettings = { |
There was a problem hiding this comment.
These are now global state. They are read from the settings registry at initialization. Then they are updated via settings.changed.connect whenever a user updates their settings.
| }; | ||
| } | ||
|
|
||
| function applySettings(setting: ISettingRegistry.ISettings): void { |
There was a problem hiding this comment.
This is the callback used by settings.changed.connect. It closes over the global state runtimeSettings and updates these values.
| ) => { | ||
| const { commands, shell } = app; | ||
|
|
||
| try { |
There was a problem hiding this comment.
Lines 205 - 211 are the new code that try to deal with when a user updates their settings.
|
bot please update snapshots |
|
Reopening to trigger the CI run after bot update |
|
Let's close the file browser (IMO preferred as we don't want changes in file browser upstream to affect our CI) or mask the date column, as it will show different time since last modification on snapshots depending on runner speed:
I think: await page.sidebar.close();should do it. |
|
Or maybe even better, let's not close it but take screenshot of only the relevant area (again to reduce maintenance cost as we would need to update and review snapshots if anything changes in UI upstream). |
|
bot please update snapshots |
|
@krassowski I'm not sure what the current CI release error is related to: Could it be related to the closing/re-opening of the PR? Or is it something I need to fix? |
Yes, it is, no need to worry about it. I mean I will have to worry about it at some point 🫠 |
krassowski
changed the title
Reference Issues or PRs
#4 and #6
What does this implement/fix?
Put a
xin the boxes that applyTesting
Documentation
Access-centered content checklist
Text styling
H1or#in markdown).Non-text content
Any other comments?