Skip to content

change smart picker behavior - #70

Open
chrip wants to merge 7 commits into
mainfrom
fix/issue-122-smart-picker-behavior
Open

change smart picker behavior#70
chrip wants to merge 7 commits into
mainfrom
fix/issue-122-smart-picker-behavior

Conversation

@chrip

@chrip chrip commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

This introduced calling smart picker by keystroke "/" anywhere in a doc, ppt, or xls.
Details are in Euro-Office/web-apps#122

For testing you need connector app and web-apps on the following branches:
https://github.com/Euro-Office/eurooffice-nextcloud/tree/fix/issue-122-smart-picker-behavior
https://github.com/Euro-Office/web-apps/tree/fix/issue-122-smart-picker-behavior

@chrip
chrip requested a review from moodyjmz June 23, 2026 08:55
@juliusknorr

Copy link
Copy Markdown
Member

I would not just trigger the modal on pressing "/". If we use this / command it should offer the providers inline in a euro-office native UI, similar to how it is integrated in Nextcloud Text.

@moodyjmz moodyjmz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both this PR and web-apps #130 need to land together — neither is functional alone. Worth coordinating the merge order or doing them in a single deploy window.

Comment thread src/editor.js Outdated
// (including the inframe 'base' layout that doesn't run the
// Assistant app's own initial-state bootstrap).
if (typeof OCA.Eurooffice.docEditor.setSmartPickerAvailable === 'function') {
OCA.Eurooffice.docEditor.setSmartPickerAvailable(true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike setAssistantAvailable above, this is unconditional — the Smart Picker is a core NC feature so no feature flag needed. Worth a short comment here so the asymmetry doesn't read as an oversight.

Comment thread src/listener.js Outdated
frame.contentWindow.focus()
}
if (typeof docEditor?.setSmartPickerCancel === 'function') {
docEditor.setSmartPickerCancel()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call depends on web-apps #130 being present. The method guard handles a missing setSmartPickerCancel gracefully, but a comment tying the two PRs together here would help future debugging.

@Alex-Arsys

Copy link
Copy Markdown

Locally tested. LGTM!

chrip added 7 commits August 18, 2026 11:02
Refs Euro-Office/web-apps#122

Works together with the Euro-Office/web-apps branch
"fix/issue-122-smart-picker-behavior" (editor-side changes). Both
branches are required for the feature to work.

- editor.js: on app ready, signal the editor that the Smart Picker is
  available via docEditor.setSmartPickerAvailable(true), so the "Add from
  Nextcloud" toolbar button is shown only when connected to Nextcloud.
- listener.js: handle picker cancellation (X/ESC may resolve empty or
  reject) by calling docEditor.setSmartPickerCancel() through the editor
  iframe and restoring window focus to it. This lets the editor remove
  the inserted "/" on selection, leave it in place on cancel, and return
  the cursor to the document.

Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
Nextcloud 35 dropped the deprecated global aliases in
"chore: remove deprecated global aliases for OC functions" (319a6497947),
which had been deprecated since 17 and slated for removal since 20.

Two of them were still in use and both threw a ReferenceError that aborted
initialisation before the editor loaded:

- oc_defaults in the page-title handler -> OC.theme
- oc_current_user in the desktop bridge -> OC.getCurrentUser().uid

Both now fall back to an empty value rather than throwing, so a future removal
degrades instead of breaking the editor.

_oc_appswebroots is unaffected: only the alias without the leading underscore
was removed, and the underscored global is still published by JSConfigHelper.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
…r frame

The editor iframe cannot reach Nextcloud itself: the Reference and TaskProcessing
OCS routes are user-scoped, carry no CORS headers, and the session cookie does not
travel cross-origin. So the editor asks, and this side performs the call with the
user's own session.

Where the request is served depends on how the editor was opened. In a frame the
Files page serves it, reaching the editor through the iframe; standalone this page
IS the top level, where window.parent is window -- relaying there posts the message
back to ourselves, which looked exactly like the picker doing nothing. One
implementation, with only the way the editor is reached passed in.

The provider list is sent to the editor rather than fetched by it, from whichever
page has a reference registry -- decided by asking the page what it has, not by
inferring frame ownership. searchProvider('') is the same call Nextcloud's own
editors use, so the menu and the picker cannot disagree about what is openable, and
the synthetic "any link" entry comes along too; it exists only client-side and no OCS
response can produce it. If a page has nothing to offer, the editor still shows its
own menu rather than a Nextcloud modal.

isInsideViewer is passed through: the picker registers its content with the Viewer's
focus trap, and without it the trap keeps pulling focus out of the modal's inputs.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
… not a wall

The Smart Picker modal covered the whole frame. Its size was never the problem --
the profile provider registers its picker element as 'normal', and
NcReferencePickerModal passes that straight to NcModal. The modal was simply
rendering with no CSS at all, and an unstyled .modal-mask/.modal-container fills
its container.

The build already emits what is needed: css-entry-points-plugin writes
css/eurooffice-editor.css as @imports of the hashed CSS chunks, 51 KB of picker
styles plus 17 KB of modal styles. Nothing ever linked it. The editor template
loaded only the hand-written css/editor.css, and the pages that add the listener
bundle added no stylesheet either -- so the modal was unstyled in both the
standalone and the in-frame case. That also explains the earlier report of the
picker looking wrong and misplaced: it was never styled, in any mode.

Add the matching addStyle next to each addScript for these two bundles. Verified
the entry stylesheet and both chunks are served (200, with content).

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
The "/" menu only ever offered "Any link" because the pages our editor runs in had
no reference providers in them. Providers reach a page only when something
dispatches RenderReferenceEvent: that is what publishes the provider list as initial
state and loads each app's picker component. We never dispatched it.

It appeared to work earlier only by accident -- the Assistant app dispatches the
event on every page it renders into, so our list was a side effect of having that app
enabled.

Dispatch it ourselves, next to loading each bundle, which is what the Text app does
in LoadEditorListener and LoadViewerListener. Three listener pages plus the editor
page, for both of its layouts: the inframe page renders as "base", but "base" still
emits scripts and initial state, and it is the page that builds the menu, so it has
to be able to answer for itself. Relying on the surrounding page only works when
there is one, which is not true when that URL is opened directly.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
Nextcloud's Assistant form had no way to put a result into our document, so results
could only be copied out by hand. It accepts actionButtons, so add an "Insert into
document" one: its output is converted from markdown to HTML and pasted, keeping
headings, lists and emphasis that a plain-text paste would drop.

Also tell that form when the Viewer is displayed. isInsideViewer decides where the
modal is mounted -- set, it is appended to body and marked .insideViewer so it lands
on top of the Viewer instead of in a stacking context underneath.

And stop assuming every picker result is a link. The Assistant entries in the Smart
Picker return the generated text itself, so inserting it as a hyperlink produced a
paragraph-long link; check whether the value parses as a URL, the same distinction
Nextcloud's own editor makes. That path became reachable when the toolbar button
started opening the full provider list.

Offer only user-facing task types. Since NC33 each type carries isInternal, and
three of the actions we exposed were internal ones: reformulation, headline and
topics. Compared with === true so servers older than NC33, which omit the field,
keep every type rather than losing all of them.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
…ger uses

Nextcloud's own Assistant form now runs the tasks, so nothing calls the operation
channel we introduced for the dialog we deleted: taskTypes, run, cancel, resolve
and fetchFile, the schedule-and-poll loop with its backoff and deadline, and the
allowlist dispatcher. Removing it also removes an editor-reachable entry point that
no longer earns its keep.

What is left of this file is the markdown to HTML conversion, which the insert path
still needs; the header now says so rather than describing a bridge that is gone.
392 lines down to 161.

Scoped to code this branch introduced. Nothing that exists in origin/main was
touched: setAssistantAvailable and the context-menu wiring in DocumentHolder.js
predate this work and keep the "Ask Nextcloud Assistant" entry working on their own.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
@chrip
chrip force-pushed the fix/issue-122-smart-picker-behavior branch from 50355f5 to a94cb8f Compare August 18, 2026 15:31
@chrip

chrip commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@moodyjmz @Alex-Arsys @juliusknorr — there are some changes in the Assistant
integration (right click menu in all editors "Ask Nextcloud Assistant). Rebased on main and cleaned up CSS handling.

Needs the editor side to work: Euro-Office/web-apps#130.

Ready for another review.

@chrip
chrip requested a review from moodyjmz September 1, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants