Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notebook jumps back to previous cell on execution #1026

Open
krassowski opened this issue Nov 26, 2023 · 3 comments
Open

Notebook jumps back to previous cell on execution #1026

krassowski opened this issue Nov 26, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@krassowski
Copy link
Member

krassowski commented Nov 26, 2023

Description

Code mirror integration has some peculiar issue which means that when a diagnostic or highlight is applied it focuses the cell. This means that transition to next cell after shift + enter may be undone.

Reproduce

This was reported upstream in jupyterlab/jupyterlab#15227

Disabling diagnostics (after a local fix for disabling logic) and disabling highlights solves it. Disabling one or the other alleviates the problem but the other part still contributes. At a first glance this looks like an upstream code mirror problem because diagnostics use @codemirror/linter while highlights use range marks so two different mechanisms give the same problem.

Expected behavior

No jumping.

@krassowski krassowski added the bug Something isn't working label Nov 26, 2023
@krassowski
Copy link
Member Author

Here is a previous bug of this nature that was filled against CM6 by yours truly: https://discuss.codemirror.net/t/how-to-add-decoration-without-focusing-the-editor-on-dispatch/6283

@krassowski
Copy link
Member Author

Ok, I do not see a downstream solution. JupyterLab 4.1 might help here because it will make it impossible to focus on the notebook node but not sure if this will fix the issue. Manual refocusing nor any attempt to partially disable the updates does not solve the issue. The way forward is to create reproducer and report this upstream.

@krassowski
Copy link
Member Author

Cannot reproduce it yet in the latest playground it is possible that we just need to update codemirror vesions.

import { Decoration, EditorView, keymap } from "@codemirror/view"
import { StateField, StateEffect } from "@codemirror/state"

const addUnderline = StateEffect.define({
  map: ({from, to}, change) => ({from: change.mapPos(from), to: change.mapPos(to)})
})
const removeMark = StateEffect.define();

const underlineField = StateField.define({
  create() {
    return Decoration.none
  },
  update(underlines, tr) {
    underlines = underlines.map(tr.changes)
    for (let e of tr.effects) {
      if (e.is(addUnderline)) {
        underlines = underlines.update({
          add: [underlineMark.range(e.value.from, e.value.to)]
        });
      } else if (e.is(removeMark)) {
        underlines = underlines.update({
          filter: (from, to, value) => {
            return false
          }
        });
      }
    }
    return underlines
  },
  provide: f => EditorView.decorations.from(f)
})

const underlineMark = Decoration.mark({class: "cm-underline"})
const underlineTheme = EditorView.baseTheme({
  ".cm-underline": { textDecoration: "underline 3px red" }
})
const views = new Set();

function underlineSelection(view) {
  let effects = view.state.selection.ranges
    .filter(r => !r.empty)
    .map(({from, to}) => addUnderline.of({from, to}))
  if (!effects.length) return false

  if (!view.state.field(underlineField, false))
    effects.push(StateEffect.appendConfig.of([underlineField,
                                              underlineTheme]))
  view.dispatch({effects});
  views.add(view);
  return true
}

function clearAllUnderlines(_view) {
  for (let view of views) {
    const effects = [removeMark.of(null)];
    view.dispatch({effects}); 
  }
  views.clear();
  return true
}

export const underlineKeymap = keymap.of([
  {
    key: "Mod-h",
    preventDefault: true,
    run: underlineSelection
  },
  {
    key: "Mod-j",
    preventDefault: true,
    run: clearAllUnderlines
  }
])


const div1 = document.createElement('div');
const div2 = document.createElement('div');
div1.style.height = '100px'
div2.style.height = '100px'
document.body.appendChild(div1);
document.body.appendChild(div2);

new EditorView({
  doc: "Select text and press Ctrl-h (Cmd-h) to add an underline\nto it.\n",
  extensions: [underlineKeymap],
  parent: div1
});

new EditorView({
  doc: "Select text and press Ctrl-h (Cmd-h) to add an underline\nto it.\n",
  extensions: [underlineKeymap],
  parent: div2
});

setTimeout(() => {
  console.log('active element before clear:', document.activeElement)
  clearAllUnderlines(null)
  console.log('active element after clear:', document.activeElement)
}, 5 * 1000)

@krassowski krassowski changed the title Jumping back to previous cell on execution Notebook jumps back to previous cell on execution Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant