Add error indicator in Table of Contents #14784

skyetim · 2023-07-02T15:11:22Z

References

#13826

Code changes

The existing indicator flow is:

when the cell is running, the status changes from idle to running; when it's scheduled, from idle to scheduled;
when the execution is complete, no matter the execution is successful or not, status goes back to idle.

The code changes update the indicator flow as:

when the cell is running, the status changes from idle to running; when it's scheduled, from idle to scheduled;
a. NEW: when an error cell is running/scheduled, the status changes from error to running/scheduled.
when the execution is complete,
a. when the execution is successful, status goes back to idle,
b. NEW: when the execution raises an error, status changes from running to error;
NEW: when an error cell's output is cleared, the status changes from error to idle.

As for the code changes,

in toc.ts, a new status is added to indicate error, a new array is added to record errorCells, onExecuted and updateRunningStatus is updated to handle flow 2b, and onOutputCleared is added to handle flow 3.
in actions.tsx a new signal is emitted when output is cleared to handle flow 3.
in toc.css, a visual is added for error status.

User-facing changes

Before:

After:

Backwards-incompatible changes

None

jupyterlab-probot · 2023-07-02T15:11:24Z

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

skyetim · 2023-07-02T16:12:26Z

bot please update galata snapshots

github-actions · 2023-07-02T17:28:08Z

Galata snapshots updated.

packages/notebook/style/toc.css

skyetim · 2023-07-09T14:10:22Z

bot please update galata snapshots

skyetim · 2023-07-19T01:51:31Z

@krassowski Bot doesn't update the snapshots. Any idea why?

krassowski · 2023-07-19T19:55:21Z

Bot, asking nicely, please update galata snapshots

krassowski · 2023-07-19T19:59:05Z

Last time it failed errored out:

  1) [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators 

    Internal error: worker process exited unexpectedly (code=null, signal=SIGABRT)

    Retry #1 ───────────────────────────────────────────────────────────────────────────────────────

    Internal error: worker process exited unexpectedly (code=null, signal=SIGABRT)

  1 failed
    [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators 
  318 passed (55.3m)
Error:   1) [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators 
    Internal error: worker process exited unexpectedly (code=null, signal=SIGABRT)
Error:   1) [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators 

    Retry #1 ───────────────────────────────────────────────────────────────────────────────────────
    Internal error: worker process exited unexpectedly (code=null, signal=SIGABRT)
Notice:   1 failed
    [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators 
  318 passed (55.3m)

Suggesting that there may be something wrong with the test.

The new run is here: https://github.com/jupyterlab/jupyterlab/actions/runs/5603502881/jobs/10250215665 but I am of little hope that it would have fixed itself - it was the only test failing.

github-actions · 2023-07-19T21:27:03Z

Galata snapshots updated.

krassowski · 2023-07-20T11:18:16Z

Kicking CI

fcollonval

Thanks @skyetim

The code looks good but I spotted the following problem:

In the screencast,

execute the all notebook
Fix the cell with an error
execute that cell
The execution indicator is briefly shown but then it goes back to error state instead of idle.

I realize also that the table of content breaks when changing cell type for the cell containing a heading - this is independent of this feature; just for awareness.

packages/notebook/src/toc.ts

krassowski · 2023-10-10T00:27:00Z

The failures are relevant:

    [jupyterlab] › test/jupyterlab/toc-running.test.ts:26:7 › ToC Running indicator › should display running indicators 
    [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators

bot please update galata snapshots

github-actions · 2023-10-10T01:33:17Z

Galata snapshots updated.

krassowski · 2023-10-12T14:35:32Z

bot please update galata snapshots

github-actions · 2023-10-12T15:50:13Z

Galata snapshots updated.

krassowski

While the code and UX is fine, the visual tests are too flaky to merge, we need to improve them to be more reproducible:

    [jupyterlab] › test/jupyterlab/toc-running.test.ts:26:7 › ToC Running indicator › should display running indicators 
    [jupyterlab] › test/jupyterlab/toc-running.test.ts:39:7 › ToC Running indicator › should display error indicators

One idea would be to use notebook upload to eliminate a source of failures due to just problems with notebook creation. Then we would need to ensure that header selection state is consistent.

skyetim · 2023-10-13T01:14:22Z

Is there a way to actually see the visual test results? I find it hard to reproduce on my local.

krassowski · 2023-10-13T08:27:00Z

Hover over a commit with failure, click on Details:
Click on Summary at the top of the page:
Scroll down, click on jupyterlab-galata-report

Unpack the zip, open the HTML with the report in a browser.

krassowski · 2023-11-06T21:11:26Z

bot please update galata snapshots

github-actions · 2023-11-06T22:09:31Z

Galata snapshots updated.

krassowski

Thanks @skyetim, all green now!

skyetim · 2023-11-07T11:00:39Z

Thanks for updating the test!

* feat: add error indicator in toc * test: for error indicator * Update Playwright Snapshots * style: make the error symbol colorblind-accessible * Update Playwright Snapshots * fix: remove from _errorCells when cell is rerun * perf: update error to -0.5 for api compatibility * test: update error status to -0.5 * Update Playwright Snapshots * Force text symbol * Update Playwright Snapshots * Update Playwright Snapshots * Update Playwright Snapshots * Revert notebook-panel-1 snapshot update * Robustify test by defining notebook a priori * Update Playwright Snapshots --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>

* Updated input field with more descriptive aria-labels and placeholders * Package integrity updates * Run integrity * Move ID generation to improve render flow * Use ellipsis character for consistency * Add error indicator in Table of Contents (#14784) * feat: add error indicator in toc * test: for error indicator * Update Playwright Snapshots * style: make the error symbol colorblind-accessible * Update Playwright Snapshots * fix: remove from _errorCells when cell is rerun * perf: update error to -0.5 for api compatibility * test: update error status to -0.5 * Update Playwright Snapshots * Force text symbol * Update Playwright Snapshots * Update Playwright Snapshots * Update Playwright Snapshots * Revert notebook-panel-1 snapshot update * Robustify test by defining notebook a priori * Update Playwright Snapshots --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Remove unnecessary requirement from servicesPlugin (#15362) * [pre-commit.ci] pre-commit autoupdate (#15358) * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0) - [github.com/python-jsonschema/check-jsonschema: 0.27.0 → 0.27.1](python-jsonschema/check-jsonschema@0.27.0...0.27.1) - [github.com/psf/black: 23.9.1 → 23.10.1](psf/black@23.9.1...23.10.1) - [github.com/astral-sh/ruff-pre-commit: v0.0.292 → v0.1.4](astral-sh/ruff-pre-commit@v0.0.292...v0.1.4) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Alig ruff version * Add ruff exception --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Update notebook window on resize if height changes (#15357) * Add a test for resizing notebook (should fail) * Update window on resize * Polish the test * Add the test notebook * Use throttler to limit the number of updates Using throttler over debouncer proposed in #15109 because debouncer would not show cells progressively during resize until user has stopped resizing. * Open files from errors (#13390) * Create renderer to render errors * WIP tests scaffold * Implement opening files from paths in error renderer * Finish sentence in a comment * Apply suggestions from code review Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Improve RFC 5147 adherence for ranges * Switch to 0-based line numbers based on RFC5147 2.2.3: "Line position counting starts with zero, so the line position before the first line of a text/plain MIME entity has the line position zero" * Fix undefined check after adding `parseInt` --------- Co-authored-by: krassowski <5832902+krassowski@users.noreply.github.com> Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Bump tj-actions/changed-files from 39.2.0 to 40.0.2 (#15342) Bumps [tj-actions/changed-files](https://github.com/tj-actions/changed-files) from 39.2.0 to 40.0.2. - [Release notes](https://github.com/tj-actions/changed-files/releases) - [Changelog](https://github.com/tj-actions/changed-files/blob/main/HISTORY.md) - [Commits](tj-actions/changed-files@v39.2.0...v40.0.2) --- updated-dependencies: - dependency-name: tj-actions/changed-files dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix scrolling past long outputs in presence of un-rendered headings (#15356) * Add a test for smooth scrolling over long outputs * Do no scroll when setting the cursor position in md cells * Adjust test name to clarify the test case * Add the test notebook * Clean up the test directory * Fix overreactive scrolling to next cell after `Shift + Enter` (#15288) * Add a test case for #14878 * Fix the smart/auto scrolling logic for long items Items larger than the size of the viewport were not considered by the scroll logic leading to bad UX when smart/auto scroll was requested on certain operations. * Use pixel-based, cell-height-derived threshold for scrolling * Implement dual behaviour, simplify code, account for top padding * Use greater or equal to avoid scrolling if item fully visible * Update tests to reflect new expectations * Clean up the temp directory after tests * Fix scrolling when dragging files in the file browser (#15318) * Fix scrolling on edges in file browser * Add a simple test * Fix update button in extension manager (#15331) * Fix update button in extension manager * Apply suggestions from code review Make action options more flexible Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Fix docs and make install options optional * Add test for update button * Update Playwright Snapshots * Revert spurious snapshot updates --------- Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: krassowski <5832902+krassowski@users.noreply.github.com> * added default property (#15346) * Exclude ipynb files in prettier pre-commit (#15378) * Update @jupyter/ydoc in dev_mode (#15383) * Define cells to run as independent of selection (#14996) * Define cells to run as independent of selection * Restore side effects of `runAll()` * Skip optional `slice` argument Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Restore previous behaviour of `runAllBelow()` by making last cell active. --------- Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> * Fix highlighting search in an out-of-viewport cell (#15376) * Fix highlighting search in an out-of-viewport cell * Update cells count * Don't show default value for objects in Settings Editor (#15380) * Don't show default value if schema type is `object` * Prettier * Bump axios from 1.3.4 to 1.6.1 (#15385) Bumps [axios](https://github.com/axios/axios) from 1.3.4 to 1.6.1. - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md) - [Commits](axios/axios@v1.3.4...v1.6.1) --- updated-dependencies: - dependency-name: axios dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix connection loop issue with standalone foreign document in LSP (#15262) * Populate * Improve VirtualDocument * Add test * Update packages/lsp/test/document.spec.ts Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> --------- Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> * Update to TypeScript 5.1 (#14638) * Bump typescript to 5.1.6 and rimraf to 5.0.5 * Dedupe the yarn.lock --------- Co-authored-by: Frédéric Collonval <fcollonval@gmail.com> * Resolved merge conflict * Package integrity updates Merged changes from main * Package integrity updates * Package integrity updates * Update Playwright Snapshots * Fixed integrity * Update Playwright Snapshots * Update Playwright Snapshots * Remove spurious snapshot updates * Update the test to reflect the new wording --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: EC2 Default User <m158261@users.noreply.github.com> Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> Co-authored-by: Tian Wang <wangtian0312@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com> Co-authored-by: Paul Kim <paulkim3151@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Divyansh Choudhary <divyanshchoudhary99@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Nate Bowditch <111072326+nbowditch-einblick@users.noreply.github.com> Co-authored-by: LJMP <24670649+LJMP@users.noreply.github.com> Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com> Co-authored-by: firai <firai.admin@gmail.com> Co-authored-by: Duc Trung Le <leductrungxf@gmail.com> Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com> Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>

feat: add error indicator in toc

9853e44

github-actions bot assigned skyetim Jul 2, 2023

github-actions bot added pkg:notebook tag:CSS For general CSS related issues and pecadilloes Design System CSS labels Jul 2, 2023

krassowski added the enhancement label Jul 2, 2023

krassowski added this to the 4.1.0 milestone Jul 2, 2023

test: for error indicator

4d72049

github-actions bot added the tag:Testing label Jul 2, 2023

Update Playwright Snapshots

854c848

skyetim closed this Jul 3, 2023

skyetim reopened this Jul 3, 2023

krassowski reviewed Jul 5, 2023

View reviewed changes

packages/notebook/style/toc.css Outdated Show resolved Hide resolved

style: make the error symbol colorblind-accessible

b87849a

skyetim closed this Jul 13, 2023

skyetim reopened this Jul 13, 2023

Update Playwright Snapshots

286f443

krassowski closed this Jul 20, 2023

krassowski reopened this Jul 20, 2023

fcollonval requested changes Jul 20, 2023

View reviewed changes

packages/notebook/src/toc.ts Outdated Show resolved Hide resolved

fix: remove from _errorCells when cell is rerun

3021bbe

fcollonval removed the status:Needs Review label Oct 8, 2023

Merge branch 'main' into toc-runningstatus-failed

0c007d5

Update Playwright Snapshots

ffcf6f3

krassowski closed this Oct 10, 2023

krassowski reopened this Oct 10, 2023

Update Playwright Snapshots

de33bcd

Revert notebook-panel-1 snapshot update

dd69520

krassowski changed the title ~~feat: add error indicator in toc~~ Add error indicator in Table of Contents Oct 12, 2023

krassowski requested changes Oct 12, 2023

View reviewed changes

JasonWeill added the status:Needs Review label Nov 1, 2023

krassowski removed the status:Needs Review label Nov 2, 2023

Robustify test by defining notebook a priori

335ec1e

Update Playwright Snapshots

ec9e8cc

krassowski closed this Nov 6, 2023

krassowski reopened this Nov 6, 2023

krassowski approved these changes Nov 7, 2023

View reviewed changes

krassowski merged commit 91c9e52 into jupyterlab:main Nov 7, 2023
77 of 79 checks passed

skyetim deleted the toc-runningstatus-failed branch November 7, 2023 14:32

fcollonval mentioned this pull request Nov 10, 2023

Show in Table of Contents where error occurred #13826

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add error indicator in Table of Contents #14784

Add error indicator in Table of Contents #14784

skyetim commented Jul 2, 2023 •

edited

jupyterlab-probot bot commented Jul 2, 2023

skyetim commented Jul 2, 2023

github-actions bot commented Jul 2, 2023

skyetim commented Jul 9, 2023

skyetim commented Jul 19, 2023

krassowski commented Jul 19, 2023

krassowski commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

krassowski commented Jul 20, 2023

fcollonval left a comment

krassowski commented Oct 10, 2023

github-actions bot commented Oct 10, 2023

krassowski commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

krassowski left a comment

skyetim commented Oct 13, 2023

krassowski commented Oct 13, 2023

krassowski commented Nov 6, 2023

github-actions bot commented Nov 6, 2023

krassowski left a comment

skyetim commented Nov 7, 2023

Add error indicator in Table of Contents #14784

Add error indicator in Table of Contents #14784

Conversation

skyetim commented Jul 2, 2023 • edited

References

Code changes

User-facing changes

Backwards-incompatible changes

jupyterlab-probot bot commented Jul 2, 2023

skyetim commented Jul 2, 2023

github-actions bot commented Jul 2, 2023

skyetim commented Jul 9, 2023

skyetim commented Jul 19, 2023

krassowski commented Jul 19, 2023

krassowski commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

krassowski commented Jul 20, 2023

fcollonval left a comment

Choose a reason for hiding this comment

krassowski commented Oct 10, 2023

github-actions bot commented Oct 10, 2023

krassowski commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

krassowski left a comment

Choose a reason for hiding this comment

skyetim commented Oct 13, 2023

krassowski commented Oct 13, 2023

krassowski commented Nov 6, 2023

github-actions bot commented Nov 6, 2023

krassowski left a comment

Choose a reason for hiding this comment

skyetim commented Nov 7, 2023

skyetim commented Jul 2, 2023 •

edited