From 43fc2bce3c826a13ac42eeaaf0a9837b739d3546 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Thu, 18 Jun 2026 17:39:45 +0200 Subject: [PATCH 01/21] fix: rename SmartPicker to "Add from Nextcloud" and change click behavior Refs https://github.com/Euro-Office/web-apps/issues/122 - Rename button caption and tooltip from "Smart Picker" / "Ask Nextcloud Assistant" to "Add from Nextcloud" in all three editors - On toolbar button click, insert "/" at cursor position before opening the picker, so the flow matches the Notes app slash-command UX - When the user selects a result, the inserted "/" is replaced by the hyperlink via pluginMethod_InputText backspace + add_Hyperlink - Replace sparkle icon with a plain "+" in the same stroke style as other toolbar icons (btn-inserthyperlink template) Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: Christoph Schaefer --- .../img/toolbar/v2/2.5x/btn-nc-assistant.svg | 8 +++----- .../documenteditor/main/app/controller/Toolbar.js | 13 +++++++++++-- apps/documenteditor/main/locale/en.json | 4 ++-- .../main/app/controller/Toolbar.js | 15 ++++++++++++--- apps/presentationeditor/main/locale/en.json | 4 ++-- .../main/app/controller/Toolbar.js | 15 ++++++++++++--- apps/spreadsheeteditor/main/locale/en.json | 4 ++-- 7 files changed, 44 insertions(+), 19 deletions(-) diff --git a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg index ab71c0a69c..eac28d4433 100644 --- a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg +++ b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg @@ -1,5 +1,3 @@ - - - \ No newline at end of file + + + diff --git a/apps/documenteditor/main/app/controller/Toolbar.js b/apps/documenteditor/main/app/controller/Toolbar.js index 1c69869ea4..96cb794416 100644 --- a/apps/documenteditor/main/app/controller/Toolbar.js +++ b/apps/documenteditor/main/app/controller/Toolbar.js @@ -1994,6 +1994,12 @@ define([ insertLink: function(data) { // gateway if (!this.api) return; + if (this._smartPickerSlashInserted) { + this._smartPickerSlashInserted = false; + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText']('', '/'); + } + } var props = new Asc.CHyperlinkProperty(); props.put_Value(data); props.put_Bookmark(null); @@ -3468,9 +3474,12 @@ define([ }, onSmartPickerClick: function() { - if (this.api && typeof this.api['asc_GetSelectedText'] === 'function') { - Common.Gateway.requestSmartPicker(this.api['asc_GetSelectedText']() || '', 'toolbar'); + if (!this.api) return; + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText']('/'); + this._smartPickerSlashInserted = true; } + Common.Gateway.requestSmartPicker('', 'toolbar'); }, onApiMathTypes: function(equation) { diff --git a/apps/documenteditor/main/locale/en.json b/apps/documenteditor/main/locale/en.json index afdde995ef..3077b3ce34 100644 --- a/apps/documenteditor/main/locale/en.json +++ b/apps/documenteditor/main/locale/en.json @@ -4033,8 +4033,8 @@ "DE.Views.Toolbar.capBtnInsShape": "Shape", "DE.Views.Toolbar.capBtnInsSmartArt": "SmartArt", "DE.Views.Toolbar.capBtnInsSymbol": "Symbol", - "DE.Views.Toolbar.capSmartPicker": "Smart Picker", - "DE.Views.Toolbar.tipSmartPicker": "Ask Nextcloud Assistant", + "DE.Views.Toolbar.capSmartPicker": "Add from Nextcloud", + "DE.Views.Toolbar.tipSmartPicker": "Add from Nextcloud", "DE.Views.Toolbar.capBtnInsTable": "Table", "DE.Views.Toolbar.capBtnInsTextart": "Text Art", "DE.Views.Toolbar.capBtnInsTextbox": "Text Box", diff --git a/apps/presentationeditor/main/app/controller/Toolbar.js b/apps/presentationeditor/main/app/controller/Toolbar.js index 50e3715370..defded54b6 100644 --- a/apps/presentationeditor/main/app/controller/Toolbar.js +++ b/apps/presentationeditor/main/app/controller/Toolbar.js @@ -1847,9 +1847,12 @@ define([ }, onSmartPickerClick: function() { - if (this.api && typeof this.api['asc_GetSelectedText'] === 'function') { - Common.Gateway.requestSmartPicker(this.api['asc_GetSelectedText']() || '', 'toolbar'); + if (!this.api) return; + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText']('/'); + this._smartPickerSlashInserted = true; } + Common.Gateway.requestSmartPicker('', 'toolbar'); }, onTablePickerSelect: function(picker, columns, rows, e) { @@ -1963,7 +1966,13 @@ define([ }, insertLink: function(data) { // gateway - + if (!this.api) return; + if (this._smartPickerSlashInserted) { + this._smartPickerSlashInserted = false; + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText']('', '/'); + } + } var props = new Asc.CHyperlinkProperty(); props.put_Value(data); props.put_Bookmark(null); diff --git a/apps/presentationeditor/main/locale/en.json b/apps/presentationeditor/main/locale/en.json index 213673b4ec..a0244ff40d 100644 --- a/apps/presentationeditor/main/locale/en.json +++ b/apps/presentationeditor/main/locale/en.json @@ -3300,8 +3300,8 @@ "PE.Views.Toolbar.capBtnInsHeaderFooter": "Header & Footer", "PE.Views.Toolbar.capBtnInsSmartArt": "SmartArt", "PE.Views.Toolbar.capBtnInsSymbol": "Symbol", - "PE.Views.Toolbar.capSmartPicker": "Smart Picker", - "PE.Views.Toolbar.tipSmartPicker": "Ask Nextcloud Assistant", + "PE.Views.Toolbar.capSmartPicker": "Add from Nextcloud", + "PE.Views.Toolbar.tipSmartPicker": "Add from Nextcloud", "PE.Views.Toolbar.capBtnSlideNum": "Slide Number", "PE.Views.Toolbar.capInsertAudio": "Audio", "PE.Views.Toolbar.capInsertChart": "Chart", diff --git a/apps/spreadsheeteditor/main/app/controller/Toolbar.js b/apps/spreadsheeteditor/main/app/controller/Toolbar.js index c3be91bd4f..17a51f2fce 100644 --- a/apps/spreadsheeteditor/main/app/controller/Toolbar.js +++ b/apps/spreadsheeteditor/main/app/controller/Toolbar.js @@ -1067,9 +1067,12 @@ define([ }, onSmartPickerClick: function() { - if (this.api && typeof this.api['asc_GetSelectedText'] === 'function') { - Common.Gateway.requestSmartPicker(this.api['asc_GetSelectedText']() || '', 'toolbar'); + if (!this.api) return; + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText']('/'); + this._smartPickerSlashInserted = true; } + Common.Gateway.requestSmartPicker('', 'toolbar'); }, onBtnPasteOptionsClick: function (btn, e) { @@ -1383,7 +1386,13 @@ define([ }, insertLink: function(data) { // gateway - + if (!this.api) return; + if (this._smartPickerSlashInserted) { + this._smartPickerSlashInserted = false; + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText']('', '/'); + } + } var props = new Asc.asc_CHyperlink(); props.asc_setHyperlinkUrl(data); props.asc_setText(data); diff --git a/apps/spreadsheeteditor/main/locale/en.json b/apps/spreadsheeteditor/main/locale/en.json index 16e04a045e..fd7c9bea65 100644 --- a/apps/spreadsheeteditor/main/locale/en.json +++ b/apps/spreadsheeteditor/main/locale/en.json @@ -4823,8 +4823,8 @@ "SSE.Views.Toolbar.capBtnInsSlicer": "Slicer", "SSE.Views.Toolbar.capBtnInsSmartArt": "SmartArt", "SSE.Views.Toolbar.capBtnInsSymbol": "Symbol", - "SSE.Views.Toolbar.capSmartPicker": "Smart Picker", - "SSE.Views.Toolbar.tipSmartPicker": "Ask Nextcloud Assistant", + "SSE.Views.Toolbar.capSmartPicker": "Add from Nextcloud", + "SSE.Views.Toolbar.tipSmartPicker": "Add from Nextcloud", "SSE.Views.Toolbar.capBtnMargins": "Margins", "SSE.Views.Toolbar.capBtnPageBreak": "Breaks", "SSE.Views.Toolbar.capBtnPageOrient": "Orientation", From 014183277b92de5b5a90d357d086b3d005be7217 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Fri, 19 Jun 2026 15:30:14 +0200 Subject: [PATCH 02/21] feat: gate "Add from Nextcloud" on connection, trigger via "/", insert at cursor Refs https://github.com/Euro-Office/web-apps/issues/122 Works together with the Euro-Office/eurooffice-nextcloud branch "fix/issue-122-smart-picker-behavior" (host-side integration). Both branches are required for the feature to work. - Show the "Add from Nextcloud" toolbar button only while connected to a Nextcloud server: the button now defaults to hidden (visible:false) and is toggled by a new setSmartPickerAvailable host command (api.js + Gateway). The "Ask Nextcloud Assistant" context-menu item keeps its existing setAssistantAvailable gating. - Trigger the picker by typing "/" in the editor body, layout-independent (e.key). In spreadsheets the keydown listener runs in the capture phase so it also fires while a cell is being edited. - Replace the inserted/typed "/" with the selected result on success and leave it in place on cancel (new setSmartPickerCancel host command). - Restore editor focus after insert and cancel via edit:complete. - Spreadsheets: insert the link as text (cell hyperlinks are whole-cell), using isCellEdited to pick pluginMethod_InputText at the cursor while editing (re-focusing the cell input) vs asc_insertInCell on a selected cell; strip the "/" trigger and never fall into the formula path. - Use a dedicated "+" icon (btn-nc-add / btn-big-nc-add) drawn in the standard 1px toolbar stroke and drop the old btn-nc-assistant sparkle. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: Christoph Schaefer --- apps/api/documents/api.js | 15 ++++ apps/common/Gateway.js | 8 ++ .../toolbar/v2/2.5x/big/btn-big-nc-add.svg | 3 + .../img/toolbar/v2/2.5x/btn-nc-add.svg | 3 + .../img/toolbar/v2/2.5x/btn-nc-assistant.svg | 3 - .../main/app/controller/Toolbar.js | 36 +++++++-- .../main/app/view/DocumentHolderExt.js | 2 +- apps/documenteditor/main/app/view/Toolbar.js | 5 +- .../main/app/controller/Toolbar.js | 38 +++++++-- .../main/app/view/DocumentHolderExt.js | 2 +- .../main/app/view/Toolbar.js | 5 +- .../main/app/controller/Toolbar.js | 78 ++++++++++++++++--- .../main/app/view/DocumentHolderExt.js | 2 +- .../main/app/view/Toolbar.js | 5 +- 14 files changed, 170 insertions(+), 35 deletions(-) create mode 100644 apps/common/main/resources/img/toolbar/v2/2.5x/big/btn-big-nc-add.svg create mode 100644 apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg delete mode 100644 apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg diff --git a/apps/api/documents/api.js b/apps/api/documents/api.js index 392bbc4168..a5164905a2 100644 --- a/apps/api/documents/api.js +++ b/apps/api/documents/api.js @@ -795,6 +795,19 @@ }); }; + var _setSmartPickerAvailable = function(data) { + _sendCommand({ + command: 'setSmartPickerAvailable', + data: data + }); + }; + + var _setSmartPickerCancel = function() { + _sendCommand({ + command: 'setSmartPickerCancel' + }); + }; + var _setMailMergeRecipients = function(data) { _sendCommand({ command: 'setMailMergeRecipients', @@ -934,6 +947,8 @@ insertLink : _insertLink, insertPlainText : _insertPlainText, setAssistantAvailable : _setAssistantAvailable, + setSmartPickerAvailable : _setSmartPickerAvailable, + setSmartPickerCancel : _setSmartPickerCancel, setMailMergeRecipients: _setMailMergeRecipients, setRevisedFile : _setRevisedFile, setFavorite : _setFavorite, diff --git a/apps/common/Gateway.js b/apps/common/Gateway.js index cde65f0fec..c0e499ad2a 100644 --- a/apps/common/Gateway.js +++ b/apps/common/Gateway.js @@ -120,6 +120,14 @@ if (window.Common === undefined) { $me.trigger('setassistantavailable', data); }, + 'setSmartPickerAvailable': function(data) { + $me.trigger('setsmartpickeravailable', data); + }, + + 'setSmartPickerCancel': function() { + $me.trigger('setsmartpickercancel'); + }, + 'setMailMergeRecipients': function(data) { $me.trigger('setmailmergerecipients', data); }, diff --git a/apps/common/main/resources/img/toolbar/v2/2.5x/big/btn-big-nc-add.svg b/apps/common/main/resources/img/toolbar/v2/2.5x/big/btn-big-nc-add.svg new file mode 100644 index 0000000000..437367377a --- /dev/null +++ b/apps/common/main/resources/img/toolbar/v2/2.5x/big/btn-big-nc-add.svg @@ -0,0 +1,3 @@ + + + diff --git a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg new file mode 100644 index 0000000000..022488797d --- /dev/null +++ b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg @@ -0,0 +1,3 @@ + + + diff --git a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg deleted file mode 100644 index eac28d4433..0000000000 --- a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/apps/documenteditor/main/app/controller/Toolbar.js b/apps/documenteditor/main/app/controller/Toolbar.js index 96cb794416..c4f4dfd114 100644 --- a/apps/documenteditor/main/app/controller/Toolbar.js +++ b/apps/documenteditor/main/app/controller/Toolbar.js @@ -253,6 +253,28 @@ define([ Common.NotificationCenter.on('app:ready', me.onAppReady.bind(me)); Common.NotificationCenter.on('app:face', me.onAppShowed.bind(me)); + Common.Gateway.on('setsmartpickeravailable', function(available) { + me._smartPickerAvailable = !!available; + me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); + }); + Common.Gateway.on('setsmartpickercancel', function() { + // Remove the "/" only if the toolbar button inserted it; a + // user-typed "/" is left in place. + if (me._smartPickerSlashArtificial && me.api && typeof me.api['pluginMethod_InputText'] === 'function') { + me.api['pluginMethod_InputText']('', '/'); + } + me._smartPickerSlashInserted = false; + me._smartPickerSlashArtificial = false; + Common.NotificationCenter.trigger('edit:complete'); + }); + $(document).on('keydown', function(e) { + var key = e.key || (e.originalEvent && e.originalEvent.key); + if (key === '/' && !e.ctrlKey && !e.metaKey && !e.altKey && me._smartPickerAvailable + && e.target && /area_id/.test(e.target.id || '')) { + me._smartPickerSlashInserted = true; + Common.Gateway.requestSmartPicker('', 'toolbar'); + } + }); }, setMode: function(mode) { @@ -1994,8 +2016,10 @@ define([ insertLink: function(data) { // gateway if (!this.api) return; + var fromSmartPicker = this._smartPickerSlashInserted; if (this._smartPickerSlashInserted) { this._smartPickerSlashInserted = false; + this._smartPickerSlashArtificial = false; if (typeof this.api['pluginMethod_InputText'] === 'function') { this.api['pluginMethod_InputText']('', '/'); } @@ -2006,6 +2030,9 @@ define([ props.put_Text(data); this.api.add_Hyperlink(props); Common.NotificationCenter.trigger('storage:link-insert', data); + if (fromSmartPicker) { + Common.NotificationCenter.trigger('edit:complete'); + } }, insertPlainText: function(data) { @@ -3478,6 +3505,7 @@ define([ if (typeof this.api['pluginMethod_InputText'] === 'function') { this.api['pluginMethod_InputText']('/'); this._smartPickerSlashInserted = true; + this._smartPickerSlashArtificial = true; } Common.Gateway.requestSmartPicker('', 'toolbar'); }, @@ -3920,11 +3948,9 @@ define([ Common.Utils.InternalSettings.set('toolbar-active-tab', !editmode && !compactview); me.toolbar.render(_.extend({isCompactView: editmode ? compactview : true}, config)); - - // Smart Picker button visibility: show only when assistant is available. - Common.Gateway.on('setassistantavailable', function(available) { - me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); - }); + if (me._smartPickerAvailable !== undefined) { + me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(me._smartPickerAvailable); + } var tab = {action: 'review', caption: me.toolbar.textTabCollaboration, dataHintTitle: 'U', layoutname: 'toolbar-collaboration'}; var $panel = me.application.getController('Common.Controllers.ReviewChanges').createToolbarPanel(); diff --git a/apps/documenteditor/main/app/view/DocumentHolderExt.js b/apps/documenteditor/main/app/view/DocumentHolderExt.js index f70c1fc85f..3b8934ffd0 100644 --- a/apps/documenteditor/main/app/view/DocumentHolderExt.js +++ b/apps/documenteditor/main/app/view/DocumentHolderExt.js @@ -2297,7 +2297,7 @@ define([], function () { caption : '--' }); me.menuParaAssistant = new Common.UI.MenuItem({ - iconCls : 'menu__icon btn-nc-assistant', + iconCls : 'menu__icon btn-nc-add', caption : me.txtNcAssistant || 'Ask Nextcloud Assistant' }); me.menuParaAssistant.assistantSeparator = menuParaAssistantSeparator; diff --git a/apps/documenteditor/main/app/view/Toolbar.js b/apps/documenteditor/main/app/view/Toolbar.js index a71b60f494..7d2009f334 100644 --- a/apps/documenteditor/main/app/view/Toolbar.js +++ b/apps/documenteditor/main/app/view/Toolbar.js @@ -1050,13 +1050,14 @@ define([ this.btnSmartPicker = new Common.UI.Button({ id: 'tlbtn-smartpicker', cls: 'btn-toolbar x-huge icon-top', - iconCls: 'toolbar__icon btn-nc-assistant', + iconCls: 'toolbar__icon btn-big-nc-add', lock: [_set.lostConnect, _set.disableOnStart, _set.viewMode], caption: me.capSmartPicker, action: 'smart-picker', dataHint: '1', dataHintDirection: 'bottom', - dataHintOffset: 'small' + dataHintOffset: 'small', + visible: false }); this.paragraphControls.push(this.btnSmartPicker); this.lockControls.push(this.btnSmartPicker); diff --git a/apps/presentationeditor/main/app/controller/Toolbar.js b/apps/presentationeditor/main/app/controller/Toolbar.js index defded54b6..d0632c5090 100644 --- a/apps/presentationeditor/main/app/controller/Toolbar.js +++ b/apps/presentationeditor/main/app/controller/Toolbar.js @@ -280,6 +280,28 @@ define([ PE.getCollection('SlideLayouts').bind({ reset: me.onResetSlides.bind(this) }); + Common.Gateway.on('setsmartpickeravailable', function(available) { + me._smartPickerAvailable = !!available; + me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); + }); + Common.Gateway.on('setsmartpickercancel', function() { + // Remove the "/" only if the toolbar button inserted it; a + // user-typed "/" is left in place. + if (me._smartPickerSlashArtificial && me.api && typeof me.api['pluginMethod_InputText'] === 'function') { + me.api['pluginMethod_InputText']('', '/'); + } + me._smartPickerSlashInserted = false; + me._smartPickerSlashArtificial = false; + Common.NotificationCenter.trigger('edit:complete'); + }); + $(document).on('keydown', function(e) { + var key = e.key || (e.originalEvent && e.originalEvent.key); + if (key === '/' && !e.ctrlKey && !e.metaKey && !e.altKey && me._smartPickerAvailable + && e.target && /area_id/.test(e.target.id || '')) { + me._smartPickerSlashInserted = true; + Common.Gateway.requestSmartPicker('', 'toolbar'); + } + }); }, setMode: function(mode) { @@ -1851,6 +1873,7 @@ define([ if (typeof this.api['pluginMethod_InputText'] === 'function') { this.api['pluginMethod_InputText']('/'); this._smartPickerSlashInserted = true; + this._smartPickerSlashArtificial = true; } Common.Gateway.requestSmartPicker('', 'toolbar'); }, @@ -1967,8 +1990,10 @@ define([ insertLink: function(data) { // gateway if (!this.api) return; + var fromSmartPicker = this._smartPickerSlashInserted; if (this._smartPickerSlashInserted) { this._smartPickerSlashInserted = false; + this._smartPickerSlashArtificial = false; if (typeof this.api['pluginMethod_InputText'] === 'function') { this.api['pluginMethod_InputText']('', '/'); } @@ -1978,8 +2003,11 @@ define([ props.put_Bookmark(null); props.put_Text(data); this.api.add_Hyperlink(props); - + Common.NotificationCenter.trigger('storage:link-insert', data); + if (fromSmartPicker) { + Common.NotificationCenter.trigger('edit:complete'); + } }, insertPlainText: function(data) { @@ -2799,11 +2827,9 @@ define([ Common.Utils.InternalSettings.set('toolbar-active-tab', !editmode && !compactview); me.toolbar.render(_.extend({compactview: editmode ? compactview : true}, config)); - - // Smart Picker button visibility: show only when assistant is available. - Common.Gateway.on('setassistantavailable', function(available) { - me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); - }); + if (me._smartPickerAvailable !== undefined) { + me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(me._smartPickerAvailable); + } var tab = {action: 'review', caption: me.toolbar.textTabCollaboration, layoutname: 'toolbar-collaboration', dataHintTitle: 'U'}; var $panel = me.getApplication().getController('Common.Controllers.ReviewChanges').createToolbarPanel(); diff --git a/apps/presentationeditor/main/app/view/DocumentHolderExt.js b/apps/presentationeditor/main/app/view/DocumentHolderExt.js index 66afccb021..327a264fad 100644 --- a/apps/presentationeditor/main/app/view/DocumentHolderExt.js +++ b/apps/presentationeditor/main/app/view/DocumentHolderExt.js @@ -1846,7 +1846,7 @@ define([], function () { caption : '--' }); me.menuParaAssistant = new Common.UI.MenuItem({ - iconCls : 'menu__icon btn-nc-assistant', + iconCls : 'menu__icon btn-nc-add', caption : me.txtNcAssistant || 'Ask Nextcloud Assistant' }); diff --git a/apps/presentationeditor/main/app/view/Toolbar.js b/apps/presentationeditor/main/app/view/Toolbar.js index b59d64650a..8ab693086e 100644 --- a/apps/presentationeditor/main/app/view/Toolbar.js +++ b/apps/presentationeditor/main/app/view/Toolbar.js @@ -1037,13 +1037,14 @@ define([ me.btnSmartPicker = new Common.UI.Button({ id: 'tlbtn-smartpicker', cls: 'btn-toolbar x-huge icon-top', - iconCls: 'toolbar__icon btn-nc-assistant', + iconCls: 'toolbar__icon btn-big-nc-add', lock: [_set.slideDeleted, _set.lostConnect, _set.noSlides, _set.disableOnStart], caption: me.capSmartPicker, action: 'smart-picker', dataHint: '1', dataHintDirection: 'bottom', - dataHintOffset: 'small' + dataHintOffset: 'small', + visible: false }); me.paragraphControls.push(me.btnSmartPicker); me.lockControls.push(me.btnSmartPicker); diff --git a/apps/spreadsheeteditor/main/app/controller/Toolbar.js b/apps/spreadsheeteditor/main/app/controller/Toolbar.js index 17a51f2fce..15dbf4ed67 100644 --- a/apps/spreadsheeteditor/main/app/controller/Toolbar.js +++ b/apps/spreadsheeteditor/main/app/controller/Toolbar.js @@ -258,11 +258,38 @@ define([ }, onLaunch: function() { + var me = this; // Create toolbar view this.toolbar = this.createView('Toolbar'); Common.NotificationCenter.on('app:ready', this.onAppReady.bind(this)); Common.NotificationCenter.on('app:face', this.onAppShowed.bind(this)); + Common.Gateway.on('setsmartpickeravailable', function(available) { + me._smartPickerAvailable = !!available; + me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); + }); + Common.Gateway.on('setsmartpickercancel', function() { + // Nothing to undo: the button doesn't pre-insert "/", and a + // user-typed "/" is left in place. Just restore focus. + me._smartPickerSlashInserted = false; + me._smartPickerSlashArtificial = false; + me._smartPickerReplace = ''; + Common.NotificationCenter.trigger('edit:complete'); + }); + // Capture phase: while editing a cell the cell editor consumes the + // keydown before it bubbles to document, so a bubble-phase listener + // never sees mid-edit "/". Capture runs first, so "/" is caught both + // on a selected cell and mid-edit (e.g. "asdf /"); _smartPickerReplace + // then drives the "/" removal on insert. + document.addEventListener('keydown', function(e) { + var key = e.key || (e.originalEvent && e.originalEvent.key); + if (key === '/' && !e.ctrlKey && !e.metaKey && !e.altKey && me._smartPickerAvailable + && e.target && /area_id/.test(e.target.id || '')) { + me._smartPickerSlashInserted = true; + me._smartPickerReplace = '/'; + Common.Gateway.requestSmartPicker('', 'toolbar'); + } + }, true); }, setMode: function(mode) { @@ -1068,10 +1095,10 @@ define([ onSmartPickerClick: function() { if (!this.api) return; - if (typeof this.api['pluginMethod_InputText'] === 'function') { - this.api['pluginMethod_InputText']('/'); - this._smartPickerSlashInserted = true; - } + // Leave any in-progress cell edit OPEN so insertLink can insert at + // the cursor (keeping the cell in edit mode for continued typing). + this._smartPickerSlashInserted = true; + this._smartPickerReplace = ''; Common.Gateway.requestSmartPicker('', 'toolbar'); }, @@ -1388,16 +1415,45 @@ define([ insertLink: function(data) { // gateway if (!this.api) return; if (this._smartPickerSlashInserted) { + var replace = this._smartPickerReplace || ''; this._smartPickerSlashInserted = false; - if (typeof this.api['pluginMethod_InputText'] === 'function') { - this.api['pluginMethod_InputText']('', '/'); + this._smartPickerSlashArtificial = false; + this._smartPickerReplace = ''; + // Insert the link as plain TEXT (a cell hyperlink is whole-cell + // and would re-link/replace the cell). NOTE: use isCellEdited, + // not asc_getCellEditMode (the latter isn't exported here). + if (this.api.isCellEdited) { + // Editing (typed "/", or mid-edit + ribbon): insert at the + // cursor and backspace the "/" trigger, keeping the cell in + // edit mode so the user can keep typing. The ribbon button + // blurs the cell-editor input, so re-focus it first or + // pluginMethod's addText won't land. (text-input path never + // adds a "+" the way asc_insertInCell while editing would.) + var areaEl = document.getElementById('area_id'); + if (areaEl && areaEl.focus) { try { areaEl.focus(); } catch (e) {} } + if (typeof this.api['pluginMethod_InputText'] === 'function') { + this.api['pluginMethod_InputText'](data, replace); + } + } else if (typeof this.api.asc_insertInCell === 'function') { + // Not editing (selected cell): append to the committed cell + // text via the value path (no formula operators, so no "+"); + // drop a trailing "/" trigger if present. + var cell = this.api.asc_getCellInfo && this.api.asc_getCellInfo(); + var cur = (cell && cell.asc_getText && cell.asc_getText()) || ''; + if (cur.slice(-1) === '/') { + cur = cur.slice(0, -1); + } + this.api.asc_insertInCell(cur + data, Asc.c_oAscPopUpSelectorType.None); } + Common.NotificationCenter.trigger('storage:link-insert', data); + Common.NotificationCenter.trigger('edit:complete'); + return; } var props = new Asc.asc_CHyperlink(); props.asc_setHyperlinkUrl(data); props.asc_setText(data); this.api.asc_insertHyperlink(props); - + Common.NotificationCenter.trigger('storage:link-insert', data); }, @@ -4983,11 +5039,9 @@ define([ } me.toolbar.render(_.extend({isCompactView: editmode ? compactview : true}, config)); - - // Smart Picker button visibility: show only when assistant is available. - Common.Gateway.on('setassistantavailable', function(available) { - me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); - }); + if (me._smartPickerAvailable !== undefined) { + me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(me._smartPickerAvailable); + } if ( !config.isEditDiagram && !config.isEditMailMerge && !config.isEditOle ) { var tab = {action: 'review', caption: me.toolbar.textTabCollaboration, layoutname: 'toolbar-collaboration', dataHintTitle: 'U'}; diff --git a/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js b/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js index bf81fbf01b..639bc7b5e2 100644 --- a/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js +++ b/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js @@ -796,7 +796,7 @@ define([], function () { caption : '--' }); me.pmiAssistant = new Common.UI.MenuItem({ - iconCls : 'menu__icon btn-nc-assistant', + iconCls : 'menu__icon btn-nc-add', caption : me.txtNcAssistant || 'Ask Nextcloud Assistant' }); diff --git a/apps/spreadsheeteditor/main/app/view/Toolbar.js b/apps/spreadsheeteditor/main/app/view/Toolbar.js index bcddadf432..5b53ef58be 100644 --- a/apps/spreadsheeteditor/main/app/view/Toolbar.js +++ b/apps/spreadsheeteditor/main/app/view/Toolbar.js @@ -1565,13 +1565,14 @@ define([ me.btnSmartPicker = new Common.UI.Button({ id: 'tlbtn-smartpicker', cls: 'btn-toolbar x-huge icon-top', - iconCls: 'toolbar__icon btn-nc-assistant', + iconCls: 'toolbar__icon btn-big-nc-add', lock: [_set.lostConnect, _set.disableOnStart, _set.viewMode], caption: me.capSmartPicker, action: 'smart-picker', dataHint: '1', dataHintDirection: 'bottom', - dataHintOffset: 'small' + dataHintOffset: 'small', + visible: false }); me.listStyles = new Common.UI.ComboDataView({ From 8a8c5f090c807b2710794aade12fa81f169bda5e Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Wed, 12 Aug 2026 17:44:27 +0200 Subject: [PATCH 03/21] feat(smartpicker): native menu at the caret, delegating to Nextcloud's pickers Typing "/" after a space or newline opens an editor-native menu at the cursor listing what Nextcloud offers to insert. Choosing an entry hands off to Nextcloud's own picker for that provider, so only the provider choice is drawn by us. The pickers themselves carry behaviour that is invisible from the outside -- minimum search lengths, per-provider result shapes, icon resolution -- and reimplementing them means rediscovering all of it one defect at a time. The provider list is pushed in by the host rather than fetched here, because a provider is only openable where its picker component is registered, and that is a fact about the host page. The list arrives as an object: Gateway relays commands through jQuery's trigger(), which spreads an array into separate handler arguments, so a bare array would arrive as its first element. Positioning is per editor. Writer and Presentation anchor on #id_target_cursor, the caret element the drawing document moves; #area_id_parent is not the caret, since sdkjs places that IME wrapper at caretBottom plus a chain of IME offsets. The spreadsheet has no text caret unless a cell is being edited inline, so it anchors on the active cell via asc_getActiveCellCoord(), as its own popups do. "/" is accepted using the same rule as Nextcloud's editors, which configure Tribute.js with requireLeadingSpace: it fires at the start of the text or after a single whitespace character. Non-character keys are ignored when tracking the previous keystroke, or a German keyboard's Shift+7 would hide the space before it. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/api/documents/api.js | 21 ++ apps/common/Gateway.js | 26 +- apps/common/main/lib/util/Assistant.js | 135 +++++++ apps/common/main/lib/view/AssistantDialog.js | 332 ++++++++++++++++++ apps/common/main/lib/view/SmartPickerMenu.js | 211 +++++++++++ .../main/resources/img/toolbar/icons.svg | 8 +- .../main/app/controller/Toolbar.js | 147 +++++++- apps/documenteditor/main/locale/en.json | 16 + .../main/resources/img/toolbar/icons.svg | 8 +- apps/documenteditor/mobile/index.html | 2 +- .../main/resources/img/toolbar/icons.svg | 8 +- .../main/app/controller/Toolbar.js | 147 +++++++- apps/presentationeditor/main/locale/en.json | 16 + .../main/resources/img/toolbar/icons.svg | 8 +- apps/presentationeditor/mobile/index.html | 2 +- .../main/app/controller/Toolbar.js | 181 +++++++++- apps/spreadsheeteditor/main/locale/en.json | 17 + .../main/resources/img/toolbar/icons.svg | 8 +- apps/spreadsheeteditor/mobile/index.html | 2 +- .../main/resources/img/toolbar/icons.svg | 8 +- apps/visioeditor/mobile/index.html | 2 +- 21 files changed, 1254 insertions(+), 51 deletions(-) create mode 100644 apps/common/main/lib/util/Assistant.js create mode 100644 apps/common/main/lib/view/AssistantDialog.js create mode 100644 apps/common/main/lib/view/SmartPickerMenu.js diff --git a/apps/api/documents/api.js b/apps/api/documents/api.js index a5164905a2..d2ecfd2acb 100644 --- a/apps/api/documents/api.js +++ b/apps/api/documents/api.js @@ -333,6 +333,7 @@ 'onUserActionRequired': // send if the user needs to enter a password or select encoding/delimiters when opening a file 'onRequestFillingStatus': // used in pdf-form fill forms mode 'onRequestSmartPicker': + 'onRequestAssistant': 'onStartFilling': // send after startFilling method, used in pdf-form editing } } @@ -802,12 +803,30 @@ }); }; + // data: {providers: [{id, title, icon_url}]}. An object, not a bare array: + // Gateway relays these through jQuery's trigger(), which spreads an array + // into separate handler arguments, so a bare array arrives as its first + // element. Every sibling command here passes an object for the same reason. + var _setSmartPickerProviders = function(data) { + _sendCommand({ + command: 'setSmartPickerProviders', + data: data + }); + }; + var _setSmartPickerCancel = function() { _sendCommand({ command: 'setSmartPickerCancel' }); }; + var _setAssistantResult = function(data) { + _sendCommand({ + command: 'setAssistantResult', + data: data + }); + }; + var _setMailMergeRecipients = function(data) { _sendCommand({ command: 'setMailMergeRecipients', @@ -948,7 +967,9 @@ insertPlainText : _insertPlainText, setAssistantAvailable : _setAssistantAvailable, setSmartPickerAvailable : _setSmartPickerAvailable, + setSmartPickerProviders : _setSmartPickerProviders, setSmartPickerCancel : _setSmartPickerCancel, + setAssistantResult : _setAssistantResult, setMailMergeRecipients: _setMailMergeRecipients, setRevisedFile : _setRevisedFile, setFavorite : _setFavorite, diff --git a/apps/common/Gateway.js b/apps/common/Gateway.js index c0e499ad2a..b8e88c9145 100644 --- a/apps/common/Gateway.js +++ b/apps/common/Gateway.js @@ -124,10 +124,18 @@ if (window.Common === undefined) { $me.trigger('setsmartpickeravailable', data); }, + 'setSmartPickerProviders': function(data) { + $me.trigger('setsmartpickerproviders', data); + }, + 'setSmartPickerCancel': function() { $me.trigger('setsmartpickercancel'); }, + 'setAssistantResult': function(data) { + $me.trigger('setassistantresult', data); + }, + 'setMailMergeRecipients': function(data) { $me.trigger('setmailmergerecipients', data); }, @@ -460,8 +468,22 @@ if (window.Common === undefined) { _postMessage({event: 'onSubmit'}); }, - requestSmartPicker: function(selectedText, source) { - _postMessage({event: 'onRequestSmartPicker', data: { selectedText: selectedText || '', source: source || 'smartpicker' }}); + // providerId targets one Nextcloud picker provider directly, so the + // editor can present the provider list itself instead of showing + // the Nextcloud provider-selection modal. + requestSmartPicker: function(selectedText, source, providerId) { + _postMessage({event: 'onRequestSmartPicker', data: { + selectedText: selectedText || '', + source: source || 'smartpicker', + providerId: providerId || '' + }}); + }, + + // Ask the Nextcloud host to perform one named operation on the user's + // behalf (see the integration's assistant.js for the allowlist). The + // host answers with setAssistantResult carrying the same id. + requestAssistant: function(id, op, params) { + _postMessage({event: 'onRequestAssistant', data: { id: id, op: op, params: params || {} }}); }, on: function(event, handler){ diff --git a/apps/common/main/lib/util/Assistant.js b/apps/common/main/lib/util/Assistant.js new file mode 100644 index 0000000000..072b85c21d --- /dev/null +++ b/apps/common/main/lib/util/Assistant.js @@ -0,0 +1,135 @@ +/* + * Nextcloud Assistant client for the editors. + * + * The editor cannot reach Nextcloud directly (the OCS routes are user-scoped and + * send no CORS headers), so every call is a named operation forwarded over the + * integration bridge and answered asynchronously by the Nextcloud page, which + * holds the user's session. + * + * Usage: + * Common.Assistant.request('taskTypes').then(...) + * Common.Assistant.run('core:text2text', {input: '…'}).then(...) + */ +define([], function () { 'use strict'; + + Common.Assistant = new(function() { + var _available = false, + _inited = false, + _seq = 0, + _pending = {}, + _taskTypes; + + /** Wire the single inbound listener the first time anything is requested. */ + var _init = function() { + if (_inited) return; + _inited = true; + + Common.Gateway.on('setassistantresult', function(reply) { + var id = reply && reply.id, + entry = id && _pending[id]; + if (!entry) return; // late answer to a discarded request + delete _pending[id]; + window.clearTimeout(entry.timer); + + if (reply.ok) { + entry.resolve(reply.data || {}); + } else { + var err = new Error(reply.error || 'The Assistant request failed.'); + err.cancelled = !!reply.cancelled; + entry.reject(err); + } + }); + }; + + /** + * The host never answering would leave the caller hanging forever. The + * host applies its own, shorter deadline to a task; this is only a + * backstop for a bridge that has gone away entirely. + */ + var BRIDGE_TIMEOUT = 300000; + + /** Marker error so callers can distinguish a user cancel from a failure. */ + var _cancelledError = function() { + var err = new Error('Cancelled'); + err.cancelled = true; + return err; + }; + + return { + /** Told by the integration whether an Assistant backend exists. */ + setAvailable: function(value) { + _available = !!value; + if (!_available) _taskTypes = undefined; + }, + + isAvailable: function() { + return _available; + }, + + /** + * Perform one named operation on the Nextcloud side. + * @param {String} op one of the operations the integration allows + * @param {Object} params operation parameters + * @return {Promise} resolves with the operation's data + */ + request: function(op, params) { + _init(); + var id = 'a' + (++_seq) + '-' + (new Date()).getTime(); + var promise = new Promise(function(resolve, reject) { + _pending[id] = { + resolve: resolve, + reject: reject, + timer: window.setTimeout(function() { + delete _pending[id]; + reject(new Error('The Assistant did not respond.')); + }, BRIDGE_TIMEOUT) + }; + }); + promise.requestId = id; + Common.Gateway.requestAssistant(id, op, params || {}); + return promise; + }, + + /** Which task types the instance actually has a provider for (cached). */ + getTaskTypes: function() { + if (!_taskTypes) { + _taskTypes = this.request('taskTypes').then(function(data) { + return data.types || {}; + }).catch(function(err) { + _taskTypes = undefined; // let a later attempt retry + throw err; + }); + } + return _taskTypes; + }, + + /** + * Schedule a task and wait for its result. + * @param {String} type Nextcloud task type id + * @param {Object} input task input map + * @return {Promise} resolves with {taskId, output, text, html} + */ + run: function(type, input) { + return this.request('run', {type: type, input: input}); + }, + + /** + * Ask the host to abandon (and server-side cancel) a running task. + * @param {String} requestId the id of the original run() request + */ + cancel: function(requestId) { + if (!requestId) return; + var entry = _pending[requestId]; + if (entry) { + delete _pending[requestId]; + window.clearTimeout(entry.timer); + entry.reject(_cancelledError()); + } + // Sent as its own request, naming the task to abandon. + this.request('cancel', {targetId: requestId}).catch(function() {}); + } + }; + })(); + + return Common.Assistant; +}); diff --git a/apps/common/main/lib/view/AssistantDialog.js b/apps/common/main/lib/view/AssistantDialog.js new file mode 100644 index 0000000000..8ec17cfdb7 --- /dev/null +++ b/apps/common/main/lib/view/AssistantDialog.js @@ -0,0 +1,332 @@ +/* + * Native Nextcloud Assistant dialog. + * + * Replaces the embedded Nextcloud modal with editor-native UI. The action list + * is built from the task types the instance actually has a provider for, so the + * dialog never offers something that would fail. + * + * The result comes back as HTML (the integration converts the model's Markdown) + * so it can be inserted with formatting intact, with plain text as a fallback. + * OK runs the action; once there is a result, OK inserts it. + */ +define([ + 'common/main/lib/component/Window', + 'common/main/lib/component/ComboBox', + 'common/main/lib/component/TextareaField', + 'common/main/lib/util/Assistant' +], function () { 'use strict'; + + Common.Views = Common.Views || {}; + + Common.Views.AssistantDialog = Common.UI.Window.extend(_.extend({ + + initialize: function(options) { + var _options = {}; + _.extend(_options, { + title: this.txtTitle, + width: 460, + height: 'auto', + cls: 'modal-dlg', + buttons: ['ok', 'cancel'] + }, options || {}); + + this.template = [ + '
', + '
', + '
', + '
', + '
', + '
', + '
' + ].join(''); + + _options.tpl = _.template(this.template)(_options); + + this.handler = _options.handler; + this.selection = _options.selection || ''; + this.taskTypes = _options.taskTypes || {}; + this._requestId = null; + this._result = null; + + Common.UI.Window.prototype.initialize.call(this, _options); + }, + + render: function() { + Common.UI.Window.prototype.render.call(this); + var me = this; + if (!me.$window) return me; + + // Delegated, and bound before anything else can throw: the footer + // buttons are re-wrapped as Common.UI.Button elsewhere, which + // replaces the nodes and would drop a handler bound directly to them. + me.$btnOk = me.$window.find('.dlg-btn[result="ok"]'); + me.$window.off('click.assistant').on('click.assistant', '.dlg-btn', function(e) { + me.onBtnClick(e); + }); + + me.cmbAction = new Common.UI.ComboBox({ + el: $('#assistant-action', me.$window), + cls: 'input-group-nr', + menuStyle: 'min-width: 100%; max-height: 210px;', + editable: false, + takeFocusOnClose: true, + data: me._buildActionList() + }); + if (me.cmbAction.store.length) { + me.cmbAction.setValue(me.cmbAction.store.at(0).get('value')); + } + + me.textInput = new Common.UI.TextareaField({ + el: $('#assistant-input', me.$window), + style: 'width: 100%; height: 110px;', + value: me.selection, + resize: false + }); + + me.$status = $('#assistant-status', me.$window); + + if (!me.cmbAction.store.length) { + me._setStatus(me.txtNoActions, true); + me.$btnOk.attr('disabled', true).addClass('disabled'); + } + + return me; + }, + + /** + * Build the action list from what the instance actually provides. + * + * Each action prefers its purpose-built Nextcloud task type: a provider + * can specialise for it, and admins enable or disable task types + * individually. Those types take a bare `input`: Text, so the editor can + * supply everything they need. Where a type has no provider on this + * instance we fall back to the free-prompt type and carry the + * instruction in the prompt, so the action still works. + * + * Types needing extra structured input are deliberately excluded until + * the dialog can collect it -- translate needs origin/target language + * enums, changetone needs a tone. + */ + _buildActionList: function() { + var types = this.taskTypes || {}, + list = [], + FREE = 'core:text2text'; + + var add = function(value, display, preferredType, prompt) { + if (types[preferredType]) { + // The task type *is* the instruction, so no prompt is needed. + list.push({value: value, displayValue: display, taskType: preferredType, prompt: ''}); + } else if (types[FREE]) { + list.push({value: value, displayValue: display, taskType: FREE, prompt: prompt}); + } + }; + + add('summarize', this.txtSummarize, 'core:text2text:summary', + 'Summarize the following text. Reply with the summary only.'); + add('rewrite', this.txtRewrite, 'core:text2text:reformulation', + 'Rewrite the following text so it reads better, keeping its meaning. Reply with the rewritten text only.'); + add('shorter', this.txtShorter, 'core:text2text:simplification', + 'Make the following text simpler and shorter, keeping its meaning. Reply with the result only.'); + add('proofread', this.txtProofread, 'core:text2text:proofread', + 'Correct spelling and grammar in the following text. Reply with the corrected text only and change nothing else.'); + add('headline', this.txtHeadline, 'core:text2text:headline', + 'Write a headline for the following text. Reply with the headline only.'); + add('topics', this.txtTopics, 'core:text2text:topics', + 'List the main topics of the following text, comma separated. Reply with the list only.'); + + // No Nextcloud task type expands text, so this one is prompt-only. + if (types[FREE]) { + list.push({value: 'longer', displayValue: this.txtLonger, taskType: FREE, + prompt: 'Expand the following text with more detail. Reply with the expanded text only.'}); + list.push({value: 'prompt', displayValue: this.txtFreePrompt, taskType: FREE, prompt: ''}); + } + + return list; + }, + + onBtnClick: function(event) { + this._handleInput(event.currentTarget.attributes['result'].value); + }, + + onPrimary: function() { + this._handleInput('ok'); + return false; + }, + + onToolClose: function() { + this._handleInput('cancel'); + }, + + _handleInput: function(state) { + if (state !== 'ok') { + this._abort(); + this.close(); + return; + } + + if (this._result) { // second OK = insert + var result = this._result; + this.close(); + if (this.handler) { + this.handler.call(this, 'insert', result); + } + return; + } + + this._run(); + }, + + _run: function() { + var me = this, + rec = me.cmbAction.getSelectedRecord(); + if (!rec) return; + + var text = me.textInput.getValue() || '', + prompt = rec.get('prompt'), + input = prompt ? (prompt + '\n\n' + text) : text; + + if (!input.replace(/\s/g, '')) { + me._setStatus(me.txtNeedInput, true); + return; + } + + me._setBusy(true); + me._setStatus(me.txtWorking, false); + + var promise = Common.Assistant.run(rec.get('taskType'), {input: input}); + me._requestId = promise.requestId; + + promise.then(function(data) { + me._requestId = null; + me._setBusy(false); + if (!data || !data.text) { + me._setStatus(me.txtNoResult, true); + return; + } + me._result = data; + me.textInput.setValue(data.text); + me._setStatus(me.txtDone, false); + me.$btnOk.text(me.txtInsert); + }).catch(function(err) { + me._requestId = null; + me._setBusy(false); + if (!err || !err.cancelled) { + me._setStatus((err && err.message) || me.txtFailed, true); + } + }); + }, + + /** Never leave a task running that nobody is waiting for. */ + _abort: function() { + if (this._requestId) { + Common.Assistant.cancel(this._requestId); + this._requestId = null; + } + }, + + close: function() { + this._abort(); + Common.UI.Window.prototype.close.apply(this, arguments); + }, + + _setBusy: function(busy) { + this.cmbAction && this.cmbAction.setDisabled(busy); + if (this.$btnOk) { + this.$btnOk.attr('disabled', !!busy); + this.$btnOk.toggleClass('disabled', !!busy); + } + }, + + _setStatus: function(message, isError) { + if (!this.$status) return; + this.$status.css('color', isError ? '#d9534f' : ''); + this.$status.text(message || ''); + }, + + txtTitle: 'Nextcloud Assistant', + txtAction: 'Action', + txtInput: 'Text', + txtSummarize: 'Summarize', + txtRewrite: 'Rewrite', + txtShorter: 'Make shorter', + txtLonger: 'Make longer', + txtProofread: 'Fix spelling & grammar', + txtFreePrompt: 'Ask the Assistant', + txtHeadline: 'Generate a headline', + txtTopics: 'Extract topics', + txtWorking: 'Working...', + txtDone: 'Ready to insert.', + txtInsert: 'Insert', + txtNoResult: 'The Assistant returned nothing.', + txtNoActions: 'No Assistant provider is configured on this server.', + txtNeedInput: 'Enter some text first.', + txtFailed: 'The Assistant request failed.' + + }, Common.Views.AssistantDialog || {})); + + /** + * Insert an Assistant result into the document. + * + * Prefers HTML so headings, lists and emphasis survive; falls back to plain + * text. pluginMethod_PasteHtml silently no-ops while a previous paste is + * still on screen (it guards on the #pmpastehtml element), so retry briefly + * rather than losing the result. + * + * @param {Object} api the editor api + * @param {Object} result {html, text} as returned by the integration + * @param {Number} attempt internal retry counter + */ + Common.Views.AssistantDialog.insertResult = function(api, result, attempt) { + if (!api || !result) return; + attempt = attempt || 0; + + var html = result.html, + text = result.text || ''; + + if (html && typeof api['pluginMethod_PasteHtml'] === 'function') { + if (document.getElementById('pmpastehtml')) { + if (attempt < 20) { + setTimeout(function() { + Common.Views.AssistantDialog.insertResult(api, result, attempt + 1); + }, 100); + return; + } + } + api['pluginMethod_PasteHtml'](html); + } else if (typeof api['pluginMethod_PasteText'] === 'function') { + api['pluginMethod_PasteText'](text); + } + + Common.NotificationCenter.trigger('edit:complete'); + }; + + /** + * Open the Assistant, having first asked the host which task types exist. + * + * @param {Object} options {api, selection} + */ + Common.Views.AssistantDialog.open = function(options) { + var api = options.api, + selection = options.selection || ''; + + Common.Assistant.getTaskTypes().then(function(types) { + var dlg = new Common.Views.AssistantDialog({ + taskTypes: types, + selection: selection, + handler: function(state, result) { + if (state === 'insert') { + Common.Views.AssistantDialog.insertResult(api, result); + } + } + }); + dlg.show(); + }).catch(function(err) { + Common.UI.warning({ + msg: (err && err.message) || 'The Nextcloud Assistant is unavailable.', + callback: function() { + Common.NotificationCenter.trigger('edit:complete'); + } + }); + }); + }; +}); diff --git a/apps/common/main/lib/view/SmartPickerMenu.js b/apps/common/main/lib/view/SmartPickerMenu.js new file mode 100644 index 0000000000..529c365f8a --- /dev/null +++ b/apps/common/main/lib/view/SmartPickerMenu.js @@ -0,0 +1,211 @@ +/* + * Native Smart Picker menu. + * + * Typing "/" after a space or newline opens this menu at the caret, listing the + * insertable providers the Nextcloud instance actually offers (files, profiles, + * Talk conversations, ...). Only this provider-selection step is editor-native; + * it is a plain list, so nothing is duplicated by drawing it ourselves. + * + * The list is pushed in by the host (setSmartPickerProviders) rather than fetched + * here. It has to come from whichever page opens the picker, because a provider is + * only openable where its picker component is registered -- and that is a fact + * about that page, invisible to the OCS endpoint we used to ask. Sourcing it any + * other way lets this menu offer entries the picker then refuses. + * + * Choosing an entry hands off to Nextcloud's own picker for that provider via + * getLinkWithPicker(), and inserts the link it returns. We deliberately do not + * reimplement those pickers. Each one carries behaviour that is not visible from + * the outside -- minimum search lengths, provider-specific result shapes, + * pagination, icon resolution -- and reimplementing it means rediscovering all of + * it by hitting the failures one at a time. + */ +define([ + 'common/main/lib/component/Menu', + 'common/main/lib/component/MenuItem' +], function () { 'use strict'; + + Common.Views = Common.Views || {}; + + Common.Views.SmartPickerMenu = new(function() { + var _menu, + _providers, + _api; + + /** + * Caret position in *viewport* coordinates. + * + * Anchors on #id_target_cursor, which is the blinking caret itself (a + * 2x13px element the drawing document moves with the cursor, declared in + * each editor's api.js). Its rect is therefore the caret, exactly. + * + * #area_id_parent was the wrong element: sdkjs places that IME wrapper at + * caretBottom + FixedPosCheckElementY + TargetOffsetY + HtmlAreaOffset + * (text_input2.js move()), an offset chain we would have to reproduce -- + * which is why the menu kept landing a constant distance too low. + * + * @return {Array|null} [left, top] in viewport coordinates + */ + var _caretPoint = function() { + var el = document.getElementById('id_target_cursor'); + if (el) { + var r = el.getBoundingClientRect(); + // Visible caret: place the menu just under it. + if (r && (r.left || r.top) && r.height >= 0) { + return [Math.round(r.left), Math.round(r.bottom + 2)]; + } + } + // Fallback: the IME wrapper. Already offset past the caret bottom by + // sdkjs, so take its top as-is. + var alt = document.getElementById('area_id_parent'); + if (alt && alt.getBoundingClientRect) { + var ar = alt.getBoundingClientRect(); + if (ar && (ar.left || ar.top)) { + return [Math.round(ar.left), Math.round(ar.top)]; + } + } + return null; + }; + + /** + * Nextcloud providers we deliberately do not list. + * + * Only the assistant_* ones, which duplicate the Assistant button and its + * native dialog. Everything else -- files, profiles, Talk, Deck, and the + * synthetic "any link" entry -- is listed and delegates to its own + * Nextcloud picker. + * + * @param {String} id provider id + * @return {Boolean} true when the entry must be hidden + */ + var _isReplaced = function(id) { + return id.indexOf('assistant_') === 0; + }; + + var _buildMenu = function(providers, onDelegate) { + var items = []; + + (providers || []).filter(function(p) { + return !_isReplaced(p.id); + }).forEach(function(p) { + items.push(new Common.UI.MenuItem({ + caption: p.title || p.id, + value: p.id, + // MenuItem renders iconImg itself as . + iconImg: p.icon_url || '' + })); + }); + + var menu = new Common.UI.Menu({ + cls: 'shifted-right', + menuAlign: 'tl-bl', + items: items + }); + menu.on('item:click', function(m, item) { + if (item && item.value) onDelegate(item.value); + }); + return menu; + }; + + return { + /** + * Show the picker menu at the caret. + * + * @param {Object} options {api, holderEl, onPick, getAnchor} + */ + show: function(options) { + var api = options.api, + holderEl = options.holderEl, + onPick = options.onPick, + getAnchor = options.getAnchor; + _api = api; + + var providers = (_providers || []).filter(function(p) { + return p && p.id && !_isReplaced(p.id); + }); + if (!providers.length) { + // Nothing pushed yet. Still show a native menu -- "/" must never + // turn into a Nextcloud modal, which is the whole point of having + // this menu. "any-link" is always openable: @nextcloud/vue resolves + // that id to its own built-in any-link picker, so the menu degrades + // to a single entry instead of a dead end or a foreign dialog. + providers = [{ + id: 'any-link', + title: Common.Views.SmartPickerMenu.txtAnyLink, + icon_url: '' + }]; + } + + // Rebuild each time: the host refreshes the list when the instance + // changes, and an admin can enable or disable apps while we are open. + if (_menu) { + _menu.hide(); + _menu = undefined; + } + _menu = _buildMenu(providers, function(providerId) { + onPick(providerId); + }); + + Common.UI.Menu.Manager.hideAll(); + + // Rebuilt every time, so the container must be emptied first: + // rendering a fresh menu into a container that still holds the + // previous one leaves both in the DOM (that was the duplicate + // list). position:fixed keeps it independent of whether the + // document holder is a positioned ancestor -- it is not. + var holder = $(holderEl), + containerId = 'menu-container-smartpicker', + container = holder.find('#' + containerId); + if (container.length) { + container.remove(); + } + container = $('
' + + '
'); + holder.append(container); + + _menu.render(container); + _menu.cmpEl.attr({tabindex: '-1'}); + + // An editor that knows better says so: the spreadsheet anchors to + // the active cell, because it has no text caret unless a cell is + // being edited inline. + var point = (getAnchor && getAnchor()) || _caretPoint(); + if (!point) { + // No caret anchor: fall back to the holder's top-left. + var hr = holder[0] ? holder[0].getBoundingClientRect() : {left: 40, top: 60}; + point = [Math.round(hr.left) + 20, Math.round(hr.top) + 20]; + } + // Keep the menu on screen near the edges. + var w = _menu.cmpEl.outerWidth() || 240, + h = _menu.cmpEl.outerHeight() || 220, + left = Math.min(point[0], Math.max(0, window.innerWidth - w - 8)), + top = point[1]; + if (top + h > window.innerHeight - 8) { + top = Math.max(8, point[1] - h - 20); // flip above the caret + } + container.css({left: left, top: top}); + + _menu.show(); + _.delay(function() { + _menu.cmpEl.focus(); + }, 10); + }, + + /** + * Receive the provider list from the host. + * + * @param {Array} providers [{id, title, icon_url}] + */ + setProviders: function(providers) { + _providers = $.isArray(providers) ? providers : []; + }, + + hide: function() { + _menu && _menu.hide(); + }, + + txtAnyLink: 'Any link' + }; + })(); + + return Common.Views.SmartPickerMenu; +}); diff --git a/apps/common/main/resources/img/toolbar/icons.svg b/apps/common/main/resources/img/toolbar/icons.svg index 62825da5b9..170b65548e 100644 --- a/apps/common/main/resources/img/toolbar/icons.svg +++ b/apps/common/main/resources/img/toolbar/icons.svg @@ -287,10 +287,8 @@ - - + + @@ -527,6 +525,8 @@ + + diff --git a/apps/documenteditor/main/app/controller/Toolbar.js b/apps/documenteditor/main/app/controller/Toolbar.js index c4f4dfd114..6bb981f398 100644 --- a/apps/documenteditor/main/app/controller/Toolbar.js +++ b/apps/documenteditor/main/app/controller/Toolbar.js @@ -34,6 +34,8 @@ define([ 'core', 'common/main/lib/component/Window', + 'common/main/lib/view/AssistantDialog', + 'common/main/lib/view/SmartPickerMenu', 'documenteditor/main/app/view/Toolbar', 'documenteditor/main/app/controller/PageLayout', ], function () { @@ -253,10 +255,19 @@ define([ Common.NotificationCenter.on('app:ready', me.onAppReady.bind(me)); Common.NotificationCenter.on('app:face', me.onAppShowed.bind(me)); + Common.Gateway.on('setassistantavailable', function(available) { + Common.Assistant.setAvailable(!!available); + }); Common.Gateway.on('setsmartpickeravailable', function(available) { me._smartPickerAvailable = !!available; me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); }); + Common.Gateway.on('setsmartpickerproviders', function(data) { + // Pushed by the host rather than fetched by us: the list has to + // come from whichever page opens the picker, and only that page + // knows which providers it can actually open. + Common.Views.SmartPickerMenu.setProviders(data && data.providers); + }); Common.Gateway.on('setsmartpickercancel', function() { // Remove the "/" only if the toolbar button inserted it; a // user-typed "/" is left in place. @@ -268,12 +279,40 @@ define([ Common.NotificationCenter.trigger('edit:complete'); }); $(document).on('keydown', function(e) { - var key = e.key || (e.originalEvent && e.originalEvent.key); - if (key === '/' && !e.ctrlKey && !e.metaKey && !e.altKey && me._smartPickerAvailable - && e.target && /area_id/.test(e.target.id || '')) { - me._smartPickerSlashInserted = true; - Common.Gateway.requestSmartPicker('', 'toolbar'); + // Remember the previous keystroke so "/" can tell whether it + // starts a word (see _slashCanTrigger). Modifier keydowns must be + // skipped: on a German layout "/" is Shift+7, so recording Shift + // would overwrite the space that actually preceded it and the + // trigger could never fire. Same for AltGr layouts. + // Only character-producing keys count as "the previous character"; + // key.length === 1 is that test. Enter is kept as a line break. + if (e.key !== '/' && (e.key.length === 1 || e.key === 'Enter')) { + me._lastTypedKey = e.key; + } + // Do not test altKey: browsers report AltGr as ctrl+alt, and on + // several layouts AltGr is how "/" is typed at all. Block only + // real shortcuts. + if (e.key !== '/' || (e.ctrlKey && !e.altKey) || e.metaKey) return; + // Permissive on purpose: sdkjs creates both #area_id (the input) + // and #area_id_main (the scrollable holder), and which one is the + // keydown target varies by editor and edit state. Anchoring this + // to /^area_id$/ silently killed the trigger. + var _tid = (e.target && e.target.id) || ''; + if (!/area_id/.test(_tid)) { + return; } + + // Like the Nextcloud editor, "/" only triggers at the start of a + // word -- after a space, a newline, or at the very beginning. + if (!me._slashCanTrigger()) { + return; + } + if (!me._smartPickerAvailable) { + return; + } + e.preventDefault(); + me.onSmartPickerMenu(); + return; }); }, @@ -2016,8 +2055,11 @@ define([ insertLink: function(data) { // gateway if (!this.api) return; - var fromSmartPicker = this._smartPickerSlashInserted; - if (this._smartPickerSlashInserted) { + // The flag is only trusted briefly: a host that dismisses the + // picker without telling us would otherwise leave it stuck on, + // and the next unrelated insertLink would backspace a real character. + var fromSmartPicker = this._isSmartPickerPending(); + if (fromSmartPicker) { this._smartPickerSlashInserted = false; this._smartPickerSlashArtificial = false; if (typeof this.api['pluginMethod_InputText'] === 'function') { @@ -3502,14 +3544,105 @@ define([ onSmartPickerClick: function() { if (!this.api) return; + // The toolbar button is the Assistant entry point (as in the + // Nextcloud editor); "/" is the Smart Picker entry point. + if (Common.Assistant.isAvailable()) { + this.onAssistantOpen(''); + return; + } + this.onSmartPickerMenu(); + return; if (typeof this.api['pluginMethod_InputText'] === 'function') { this.api['pluginMethod_InputText']('/'); this._smartPickerSlashInserted = true; + this._smartPickerAt = (new Date()).getTime(); this._smartPickerSlashArtificial = true; } Common.Gateway.requestSmartPicker('', 'toolbar'); }, + + /** + * Whether "/" should open the Smart Picker at the current position. + * + * Mirrors how Nextcloud does it. NcRichContenteditable configures + * Tribute.js with {trigger: '/', requireLeadingSpace: true, + * allowSpaces: false}, and Tribute's rule (tribute.min.js) is: + * + * m >= 0 && (0 === m || !requireLeadingSpace + * || /[\xA0\s]/g.test(textBeforeCaret.substring(m-1, m))) + * + * i.e. it fires at the start of the text, or when the single character + * before the trigger is whitespace. Tribute reads that from the *text*, + * which is why it is layout-independent. The editor exposes no cheap way + * to read the character before the caret, so this approximates it with the + * last character-producing keystroke: key.length === 1 is exactly that + * test and excludes Shift/Alt/AltGraph/arrows/F-keys without maintaining a + * list. Tracking every key was the original bug -- on a German layout "/" + * is Shift+7, so Shift overwrote the space that preceded it. + * + * @return {Boolean} true when "/" should open the picker + */ + _slashCanTrigger: function() { + var prev = this._lastTypedKey; + if (prev === undefined) return true; // nothing typed yet + if (prev === 'Enter') return true; // start of a new line + if (prev.length === 1 && /[\xA0\s]/.test(prev)) return true; + return false; + }, + + /** Open the native Smart Picker menu at the caret. */ + onSmartPickerMenu: function() { + var me = this; + if (!me.api) return; + // Never fail silently here: this runs from a keystroke, so a thrown + // error would look to the user like "/" simply does nothing. + try { + Common.Views.SmartPickerMenu.show({ + api: me.api, + holderEl: $('#editor_sdk'), + onPick: function(providerId) { + me._smartPickerSlashInserted = true; + me._smartPickerAt = (new Date()).getTime(); + Common.Gateway.requestSmartPicker('', 'toolbar', providerId); + } + }); + } catch (err) { + Common.UI.warning({msg: 'Smart Picker failed to open: ' + (err && err.message)}); + } + }, + + /** + * Whether a smart-picker round-trip is genuinely still in progress. + * + * @return {Boolean} true while the flag is fresh + */ + _isSmartPickerPending: function() { + if (!this._smartPickerSlashInserted) return false; + var age = (new Date()).getTime() - (this._smartPickerAt || 0); + if (age > 120000) { + this._smartPickerSlashInserted = false; + this._smartPickerSlashArtificial = false; + this._smartPickerReplace = ''; + return false; + } + return true; + }, + + /** + * Open the native Assistant dialog, seeded with the current selection. + * + * @param {String} selected text to work on, or '' to read the selection + */ + onAssistantOpen: function(selected) { + if (!this.api) return; + var text = selected; + if (!text && typeof this.api.asc_GetSelectedText === 'function') { + try { text = this.api.asc_GetSelectedText() || ''; } catch (e) { text = ''; } + } + Common.Views.AssistantDialog.open({api: this.api, selection: text}); + }, + onApiMathTypes: function(equation) { this._equationTemp = equation; var me = this; diff --git a/apps/documenteditor/main/locale/en.json b/apps/documenteditor/main/locale/en.json index 3077b3ce34..91e4bc3b14 100644 --- a/apps/documenteditor/main/locale/en.json +++ b/apps/documenteditor/main/locale/en.json @@ -394,6 +394,22 @@ "Common.Controllers.Shortcuts.txtLabelZoom100": "Zoom100", "Common.Controllers.Shortcuts.txtLabelZoomIn": "ZoomIn", "Common.Controllers.Shortcuts.txtLabelZoomOut": "ZoomOut", + "Common.Views.AssistantDialog.txtAction": "Action", + "Common.Views.AssistantDialog.txtDone": "Ready to insert.", + "Common.Views.AssistantDialog.txtFailed": "The Assistant request failed.", + "Common.Views.AssistantDialog.txtFreePrompt": "Ask the Assistant", + "Common.Views.AssistantDialog.txtInput": "Text", + "Common.Views.AssistantDialog.txtInsert": "Insert", + "Common.Views.AssistantDialog.txtLonger": "Make longer", + "Common.Views.AssistantDialog.txtNeedInput": "Enter some text first.", + "Common.Views.AssistantDialog.txtNoActions": "No Assistant provider is configured on this server.", + "Common.Views.AssistantDialog.txtNoResult": "The Assistant returned nothing.", + "Common.Views.AssistantDialog.txtProofread": "Fix spelling & grammar", + "Common.Views.AssistantDialog.txtRewrite": "Rewrite", + "Common.Views.AssistantDialog.txtShorter": "Make shorter", + "Common.Views.AssistantDialog.txtSummarize": "Summarize", + "Common.Views.AssistantDialog.txtTitle": "Nextcloud Assistant", + "Common.Views.AssistantDialog.txtWorking": "Working...", "Common.define.chartData.textArea": "Area", "Common.define.chartData.textAreaStacked": "Stacked area", "Common.define.chartData.textAreaStackedPer": "100% Stacked area", diff --git a/apps/documenteditor/main/resources/img/toolbar/icons.svg b/apps/documenteditor/main/resources/img/toolbar/icons.svg index 550930cee1..dc814e9b3e 100644 --- a/apps/documenteditor/main/resources/img/toolbar/icons.svg +++ b/apps/documenteditor/main/resources/img/toolbar/icons.svg @@ -287,10 +287,8 @@ - - + + @@ -527,6 +525,8 @@ + + diff --git a/apps/documenteditor/mobile/index.html b/apps/documenteditor/mobile/index.html index 6e4b83eaf2..ac6874c2cc 100644 --- a/apps/documenteditor/mobile/index.html +++ b/apps/documenteditor/mobile/index.html @@ -218,7 +218,7 @@ width: 24px; height: 24px; fill: var(--skl-toolbar-icons); -}'), ''); + }); + + it('drops non-image and script-bearing schemes', function () { + assert.strictEqual(clean('javascript:alert(1)'), ''); + assert.strictEqual(clean('data:text/html;base64,PHNjcmlwdD4='), ''); + assert.strictEqual(clean('//evil.example/i.png'), ''); + }); + + it('drops non-strings and empty values', function () { + assert.strictEqual(clean(undefined), ''); + assert.strictEqual(clean(null), ''); + assert.strictEqual(clean(''), ''); + assert.strictEqual(clean({}), ''); + }); + }); + + describe('createPending', function () { + + it('reports nothing outstanding before a request', function () { + var pending = SmartPicker.createPending(); + assert.strictEqual(pending.isPending(), false); + // null, not '': an unrelated insertLink must delete nothing. + assert.strictEqual(pending.consume(), null); + }); + + it('returns the text to delete, exactly once', function () { + var pending = SmartPicker.createPending(); + pending.begin('/fil'); + assert.strictEqual(pending.isPending(), true); + assert.strictEqual(pending.consume(), '/fil'); + assert.strictEqual(pending.consume(), null); + }); + + it('deletes nothing after a cancelled request', function () { + var pending = SmartPicker.createPending(); + pending.begin('/fil'); + pending.clear(); + assert.strictEqual(pending.consume(), null); + }); + + it('expires a stale request instead of eating a character', function () { + var pending = SmartPicker.createPending(); + var realNow = Date.now; + try { + var now = 1000000; + Date.now = function () { return now; }; + pending.begin('/fil'); + now += SmartPicker.PENDING_TIMEOUT + 1; + assert.strictEqual(pending.isPending(), false); + assert.strictEqual(pending.consume(), null); + } finally { + Date.now = realNow; + } + }); + }); + }); +})); From 4760b6b513dcaad8f74f7afb6db82b1a97e1dba0 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Fri, 14 Aug 2026 08:35:03 +0200 Subject: [PATCH 08/21] chore(license): correct the copyright header on the files this branch adds The four files this branch adds carried the Ascensio System SIA header, copied from neighbouring upstream files to satisfy the "every file needs a licence header" rule. That got the licence right and the copyright holder wrong: it credited Ascensio for code they did not write, and asserted their Section 7 additional terms -- the non-infringement warranty exclusion and the CC-BY-SA clause on GUI elements -- over Nextcloud-authored work. Those are Ascensio's terms to place on Ascensio's code. Replaced with the SPDX header this repository already uses for exactly this case, in apps/spreadsheeteditor/main/app/view/CheckBoxSettingsDialog.js: /*! * SPDX-FileCopyrightText: 2026 Nextcloud GmbH or an Nextcloud affiliate company and Euro-Office contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ AGPL-3.0-or-later matches LICENSE.txt, so the licence itself is unchanged. The /*! form is deliberate -- terser keeps bang comments and strips plain ones, so the notice survives into the built bundle. Affects only files this branch adds: apps/common/main/lib/util/AssistantInsert.js apps/common/main/lib/util/SmartPicker.js apps/common/main/lib/view/SmartPickerMenu.js test/unit-tests/common/main/lib/util/SmartPicker.js Upstream files this branch modifies keep their original Ascensio headers. The two SVGs it adds stay bare, as every other asset in the repository is. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/lib/util/AssistantInsert.js | 26 +++---------------- apps/common/main/lib/util/SmartPicker.js | 26 +++---------------- apps/common/main/lib/view/SmartPickerMenu.js | 26 +++---------------- .../common/main/lib/util/SmartPicker.js | 26 +++---------------- 4 files changed, 12 insertions(+), 92 deletions(-) diff --git a/apps/common/main/lib/util/AssistantInsert.js b/apps/common/main/lib/util/AssistantInsert.js index 7645cf5ca0..35db8c906f 100644 --- a/apps/common/main/lib/util/AssistantInsert.js +++ b/apps/common/main/lib/util/AssistantInsert.js @@ -1,26 +1,6 @@ -/* - * (c) Copyright Ascensio System SIA 2010-2024 - * - * This program is a free software product. You can redistribute it and/or - * modify it under the terms of the GNU Affero General Public License (AGPL) - * version 3 as published by the Free Software Foundation. In accordance with - * Section 7(a) of the GNU AGPL its Section 15 shall be amended to the effect - * that Ascensio System SIA expressly excludes the warranty of non-infringement - * of any third-party rights. - * - * This program is distributed WITHOUT ANY WARRANTY; without even the implied - * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. For - * details, see the GNU AGPL at: http://www.gnu.org/licenses/agpl-3.0.html - * - * The interactive user interfaces in modified source and object code versions - * of the Program must display Appropriate Legal Notices, as required under - * Section 5 of the GNU AGPL version 3. - * - * All the Product's GUI elements, including illustrations and icon sets, as - * well as technical writing content are licensed under the terms of the - * Creative Commons Attribution-ShareAlike 4.0 International. See the License - * terms at http://creativecommons.org/licenses/by-sa/4.0/legalcode - * +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH or an Nextcloud affiliate company and Euro-Office contributors + * SPDX-License-Identifier: AGPL-3.0-or-later */ /* diff --git a/apps/common/main/lib/util/SmartPicker.js b/apps/common/main/lib/util/SmartPicker.js index ae0aa099a8..6e64ff695a 100644 --- a/apps/common/main/lib/util/SmartPicker.js +++ b/apps/common/main/lib/util/SmartPicker.js @@ -1,26 +1,6 @@ -/* - * (c) Copyright Ascensio System SIA 2010-2024 - * - * This program is a free software product. You can redistribute it and/or - * modify it under the terms of the GNU Affero General Public License (AGPL) - * version 3 as published by the Free Software Foundation. In accordance with - * Section 7(a) of the GNU AGPL its Section 15 shall be amended to the effect - * that Ascensio System SIA expressly excludes the warranty of non-infringement - * of any third-party rights. - * - * This program is distributed WITHOUT ANY WARRANTY; without even the implied - * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. For - * details, see the GNU AGPL at: http://www.gnu.org/licenses/agpl-3.0.html - * - * The interactive user interfaces in modified source and object code versions - * of the Program must display Appropriate Legal Notices, as required under - * Section 5 of the GNU AGPL version 3. - * - * All the Product's GUI elements, including illustrations and icon sets, as - * well as technical writing content are licensed under the terms of the - * Creative Commons Attribution-ShareAlike 4.0 International. See the License - * terms at http://creativecommons.org/licenses/by-sa/4.0/legalcode - * +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH or an Nextcloud affiliate company and Euro-Office contributors + * SPDX-License-Identifier: AGPL-3.0-or-later */ /* diff --git a/apps/common/main/lib/view/SmartPickerMenu.js b/apps/common/main/lib/view/SmartPickerMenu.js index dcfebf50a9..fdcc5fa265 100644 --- a/apps/common/main/lib/view/SmartPickerMenu.js +++ b/apps/common/main/lib/view/SmartPickerMenu.js @@ -1,26 +1,6 @@ -/* - * (c) Copyright Ascensio System SIA 2010-2024 - * - * This program is a free software product. You can redistribute it and/or - * modify it under the terms of the GNU Affero General Public License (AGPL) - * version 3 as published by the Free Software Foundation. In accordance with - * Section 7(a) of the GNU AGPL its Section 15 shall be amended to the effect - * that Ascensio System SIA expressly excludes the warranty of non-infringement - * of any third-party rights. - * - * This program is distributed WITHOUT ANY WARRANTY; without even the implied - * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. For - * details, see the GNU AGPL at: http://www.gnu.org/licenses/agpl-3.0.html - * - * The interactive user interfaces in modified source and object code versions - * of the Program must display Appropriate Legal Notices, as required under - * Section 5 of the GNU AGPL version 3. - * - * All the Product's GUI elements, including illustrations and icon sets, as - * well as technical writing content are licensed under the terms of the - * Creative Commons Attribution-ShareAlike 4.0 International. See the License - * terms at http://creativecommons.org/licenses/by-sa/4.0/legalcode - * +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH or an Nextcloud affiliate company and Euro-Office contributors + * SPDX-License-Identifier: AGPL-3.0-or-later */ /* diff --git a/test/unit-tests/common/main/lib/util/SmartPicker.js b/test/unit-tests/common/main/lib/util/SmartPicker.js index 0fc0eb5957..26addcc5ea 100644 --- a/test/unit-tests/common/main/lib/util/SmartPicker.js +++ b/test/unit-tests/common/main/lib/util/SmartPicker.js @@ -1,26 +1,6 @@ -/* - * (c) Copyright Ascensio System SIA 2010-2024 - * - * This program is a free software product. You can redistribute it and/or - * modify it under the terms of the GNU Affero General Public License (AGPL) - * version 3 as published by the Free Software Foundation. In accordance with - * Section 7(a) of the GNU AGPL its Section 15 shall be amended to the effect - * that Ascensio System SIA expressly excludes the warranty of non-infringement - * of any third-party rights. - * - * This program is distributed WITHOUT ANY WARRANTY; without even the implied - * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. For - * details, see the GNU AGPL at: http://www.gnu.org/licenses/agpl-3.0.html - * - * The interactive user interfaces in modified source and object code versions - * of the Program must display Appropriate Legal Notices, as required under - * Section 5 of the GNU AGPL version 3. - * - * All the Product's GUI elements, including illustrations and icon sets, as - * well as technical writing content are licensed under the terms of the - * Creative Commons Attribution-ShareAlike 4.0 International. See the License - * terms at http://creativecommons.org/licenses/by-sa/4.0/legalcode - * +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH or an Nextcloud affiliate company and Euro-Office contributors + * SPDX-License-Identifier: AGPL-3.0-or-later */ /** From 8df5d28e07c06cd828b8200254bb6a1aacbbf5c6 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Fri, 14 Aug 2026 16:07:46 +0200 Subject: [PATCH 09/21] test(smart-picker): register the SmartPicker suite in the test runner The suite file arrives with this branch, so its registration belongs here too rather than in the harness fix (#194), which now stands on its own. Note that the runner itself only works once #194 lands -- every path in this file still points at a directory that is not in the repository. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- test/unit-tests/common/index.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit-tests/common/index.html b/test/unit-tests/common/index.html index 212fbb75ec..93898f36be 100644 --- a/test/unit-tests/common/index.html +++ b/test/unit-tests/common/index.html @@ -101,7 +101,8 @@ require(['../test/unit-tests/common'], function () { require([ './main/lib/util/utils.js', - './main/lib/component/Button.js' + './main/lib/component/Button.js', + './main/lib/util/SmartPicker.js' ], runMocha); }); }); From 63ab934d631d31466a3bea369a66b2631cdc9417 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Fri, 14 Aug 2026 23:18:33 +0200 Subject: [PATCH 10/21] fix(smartpicker): let the pending record outlast a person using the picker PENDING_TIMEOUT was one minute, which is inside the time an ordinary interaction takes. Picking a provider starts the record; the reply consumes it and deletes the "/query" the user typed. In between, the host's picker is a modal in front of the editor, so nothing clears the record -- and expiring there does not fail safe: insertLink still inserts the link, but consume() has already returned null, so the trigger text is left in the document beside it. Measured against a running editor, same clicks in the same order: 11 s from picking a provider to confirming -> "Hello /prohttp://..." became "Hello http://..." correct 77 s -> "Hello /prohttp://..." wrong Raised to ten minutes rather than removing the check, because the check is still the documented backstop for a host that neither answers nor cancels. Expiring is only protective if the document moved without a keystroke, which onActivity already covers for the keyboard, and the caret cannot move on its own -- so a generous value gives up nothing that was actually being guarded. The new test pins the failure: it passes at ten minutes and fails at one, and the existing stale-request test is written relative to the constant, so it keeps its meaning either way. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/lib/util/SmartPicker.js | 22 +++++++++++++++---- .../common/main/lib/util/SmartPicker.js | 21 ++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/apps/common/main/lib/util/SmartPicker.js b/apps/common/main/lib/util/SmartPicker.js index 6e64ff695a..00c611790a 100644 --- a/apps/common/main/lib/util/SmartPicker.js +++ b/apps/common/main/lib/util/SmartPicker.js @@ -43,11 +43,25 @@ define([], function () { 'use strict'; * How long a request to the host may stay outstanding. * * A backstop, not the primary defence: any keystroke in the editor area - * clears the flag, and the user cannot type there while the host's - * picker is open. This only covers a host that neither answers nor - * cancels while the user also never touches the keyboard again. + * clears the flag (see onActivity), and the user cannot type there while + * the host's picker is open. This only covers a host that neither answers + * nor cancels while the user also never touches the keyboard again. + * + * It therefore has to outlast a person using the picker, because nothing + * else clears the record while that modal is in front of the editor. One + * minute did not: searching a name, scrolling a longer result list, or + * simply being interrupted takes longer than that, and expiring mid-flow + * does not fail safe -- insertLink still inserts the link, but consume() + * has already returned null, so the "/query" the user typed is left in + * the document beside it. Measured before this was raised: 11 s from + * picking a provider to confirming replaced the trigger correctly, 77 s + * left it behind. + * + * Expiring is only protective if the document moved under us without a + * keystroke, which the caret cannot do on its own -- so a generous value + * costs nothing that the keystroke clear was not already covering. */ - PENDING_TIMEOUT: 60000, + PENDING_TIMEOUT: 600000, /** * Whether "/" should open the Smart Picker at the current position. diff --git a/test/unit-tests/common/main/lib/util/SmartPicker.js b/test/unit-tests/common/main/lib/util/SmartPicker.js index 26addcc5ea..8f27b5fd45 100644 --- a/test/unit-tests/common/main/lib/util/SmartPicker.js +++ b/test/unit-tests/common/main/lib/util/SmartPicker.js @@ -149,6 +149,27 @@ assert.strictEqual(pending.consume(), null); }); + it('survives a person taking their time in the picker', function () { + // The failure this pins down: the picker is a modal in front of + // the editor, so nothing clears the record while it is open, and + // expiring mid-flow does not fail safe -- the link still gets + // inserted while the trigger text stays in the document. Two + // minutes is an ordinary amount of time to spend searching a name + // or being interrupted; it used to be past the limit. + var pending = SmartPicker.createPending(); + var realNow = Date.now; + try { + var now = 1000000; + Date.now = function () { return now; }; + pending.begin('/pro'); + now += 120000; + assert.strictEqual(pending.isPending(), true); + assert.strictEqual(pending.consume(), '/pro'); + } finally { + Date.now = realNow; + } + }); + it('expires a stale request instead of eating a character', function () { var pending = SmartPicker.createPending(); var realNow = Date.now; From 27be4c714e3013de36c00d7a44f858b11ef8bd29 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Sat, 15 Aug 2026 00:01:10 +0200 Subject: [PATCH 11/21] fix(smartpicker): do not delete the trigger unless it is still there pluginMethod_InputText(text, textReplace) does not match textReplace against the document. It fires textReplace.length backspaces at the caret and inserts. Verified against a running editor: with "Hello world" and the caret at the end, asking it to replace "ZZZZ" -- a string that appears nowhere -- left "Hello w". So the deletion is only correct while the caret still sits right after the text the user typed, and co-editing reaches that state without the user doing anything: a remote change shifts this user's caret, and onActivity does not fire because a remote change is not a keystroke here. The trigger is also plain text in the shared document while the picker is open, so a co-author may tidy away what looks like a typo. Raising PENDING_TIMEOUT widened that window, which is what prompted looking at it. Failure is not symmetric: a stray "/query" left behind is cosmetic and the user can delete it, while eating four characters of someone else's sentence is data loss that syncs to everyone. So triggerStillThere permits the deletion only when the word before the caret still matches what was typed, and the link is inserted either way. It compares the query rather than the whole trigger because "/" is punctuation and so a word boundary: after "Hello /pro" the word part before the caret is "pro". A bare "/" expects "", which is what a caret after punctuation gives. asc_GetCurrentWord is exported by word/api.js only, so Presentation and Spreadsheet cannot be asked and keep the previous behaviour rather than lose the feature -- exporting it there would extend the guard to them unchanged. The guard also permits the deletion if the probe throws; a diagnostic must not stop an insertion. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/lib/util/SmartPicker.js | 50 +++++++++++++++++++ .../main/app/controller/Toolbar.js | 5 ++ .../main/app/controller/Toolbar.js | 5 ++ .../main/app/controller/Toolbar.js | 5 ++ .../common/main/lib/util/SmartPicker.js | 40 +++++++++++++++ 5 files changed, 105 insertions(+) diff --git a/apps/common/main/lib/util/SmartPicker.js b/apps/common/main/lib/util/SmartPicker.js index 00c611790a..71a1c4aecf 100644 --- a/apps/common/main/lib/util/SmartPicker.js +++ b/apps/common/main/lib/util/SmartPicker.js @@ -63,6 +63,56 @@ define([], function () { 'use strict'; */ PENDING_TIMEOUT: 600000, + /** + * Whether the trigger text is still sitting where it was typed. + * + * The deletion primitive cannot answer this. pluginMethod_InputText(text, + * textReplace) does not match textReplace against the document at all -- it + * fires textReplace.length backspaces at the caret and inserts. Verified + * against a running editor: with "Hello world" and the caret at the end, + * asking it to replace "ZZZZ" -- a string that appears nowhere -- left + * "Hello w". + * + * So anything that moves the caret between picking a provider and the reply + * arriving turns the deletion into silent damage, and co-editing makes that + * reachable without the user doing anything: a remote change shifts this + * user's caret, and onActivity does not fire because a remote change is not + * a keystroke here. The trigger is also visible to everyone as literal text + * while the picker is open, so a co-author may simply tidy it away. + * + * Failure is not symmetric. Leaving a stray "/query" behind is cosmetic and + * the user can delete it; eating four characters of someone else's sentence + * is data loss that syncs to everyone. So this only permits the deletion + * when the text before the caret still looks like what was typed. + * + * It compares against the query rather than the whole trigger because the + * leading "/" is punctuation and therefore a word boundary: after "Hello + * /pro" the word part before the caret is "pro". A trigger of just "/" + * expects "", which is what a caret sitting right after punctuation gives. + * + * Editors whose api does not expose asc_GetCurrentWord (Presentation and + * Spreadsheet at the time of writing -- it is exported only in word/api.js) + * cannot be checked, and keep the previous behaviour rather than losing the + * feature. Exporting it there would extend this guard to them unchanged. + * + * @param {Object} api the editor api + * @param {String} replace the trigger text, "/" plus the query + * @return {Boolean} true when deleting it is safe, or cannot be checked + */ + triggerStillThere: function (api, replace) { + if (!api || typeof api.asc_GetCurrentWord !== 'function') return true; + + var expected = (replace || '').replace(/^\//, ''); + var actual; + try { + actual = api.asc_GetCurrentWord(-1); + } catch (e) { + // Never let a probe stop an insertion; fall back to the old behaviour. + return true; + } + return (actual || '') === expected; + }, + /** * Whether "/" should open the Smart Picker at the current position. * diff --git a/apps/documenteditor/main/app/controller/Toolbar.js b/apps/documenteditor/main/app/controller/Toolbar.js index e0a745adb4..63236288bd 100644 --- a/apps/documenteditor/main/app/controller/Toolbar.js +++ b/apps/documenteditor/main/app/controller/Toolbar.js @@ -2034,6 +2034,11 @@ define([ // null when this reply is not the answer to our request, in which // case nothing must be deleted -- see Common.Utils.SmartPicker. var replace = this._smartPicker.consume(); + // Only if it is still there; the primitive deletes blind. See + // Common.Utils.SmartPicker.triggerStillThere. + if (replace && !Common.Utils.SmartPicker.triggerStillThere(this.api, replace)) { + replace = null; + } if (replace && typeof this.api['pluginMethod_InputText'] === 'function') { // Delete the "/" the user typed to open the menu. It really is // in the document: the trigger does not cancel the keystroke. diff --git a/apps/presentationeditor/main/app/controller/Toolbar.js b/apps/presentationeditor/main/app/controller/Toolbar.js index 609690db68..5f81b98d19 100644 --- a/apps/presentationeditor/main/app/controller/Toolbar.js +++ b/apps/presentationeditor/main/app/controller/Toolbar.js @@ -2011,6 +2011,11 @@ define([ // null when this reply is not the answer to our request, in which // case nothing must be deleted -- see Common.Utils.SmartPicker. var replace = this._smartPicker.consume(); + // Only if it is still there; the primitive deletes blind. See + // Common.Utils.SmartPicker.triggerStillThere. + if (replace && !Common.Utils.SmartPicker.triggerStillThere(this.api, replace)) { + replace = null; + } if (replace && typeof this.api['pluginMethod_InputText'] === 'function') { // Delete the "/" and the query typed after it. Both really are // in the document: the trigger does not cancel the keystrokes. diff --git a/apps/spreadsheeteditor/main/app/controller/Toolbar.js b/apps/spreadsheeteditor/main/app/controller/Toolbar.js index 347ea79cb9..f172642635 100644 --- a/apps/spreadsheeteditor/main/app/controller/Toolbar.js +++ b/apps/spreadsheeteditor/main/app/controller/Toolbar.js @@ -1456,6 +1456,11 @@ define([ // null when this reply is not the answer to our request, in which // case nothing must be deleted -- see Common.Utils.SmartPicker. var replace = this._smartPicker.consume(); + // Only if it is still there; the primitive deletes blind. See + // Common.Utils.SmartPicker.triggerStillThere. + if (replace && !Common.Utils.SmartPicker.triggerStillThere(this.api, replace)) { + replace = null; + } if (replace !== null) { // Insert the link as plain TEXT (a cell hyperlink is whole-cell // and would re-link/replace the cell). NOTE: use isCellEdited, diff --git a/test/unit-tests/common/main/lib/util/SmartPicker.js b/test/unit-tests/common/main/lib/util/SmartPicker.js index 8f27b5fd45..5b6be2293d 100644 --- a/test/unit-tests/common/main/lib/util/SmartPicker.js +++ b/test/unit-tests/common/main/lib/util/SmartPicker.js @@ -125,6 +125,46 @@ }); }); + describe('triggerStillThere', function () { + + // The word part before the caret, which is what asc_GetCurrentWord(-1) + // returns. "/" is punctuation, so it is not part of that word. + var apiWith = function (wordBeforeCaret) { + return {asc_GetCurrentWord: function () { return wordBeforeCaret; }}; + }; + + it('permits the deletion when the query is still before the caret', function () { + assert.strictEqual(SmartPicker.triggerStillThere(apiWith('pro'), '/pro'), true); + }); + + it('refuses when something else is there now', function () { + // A co-editor's change, or the user clicking elsewhere: deleting + // here would eat characters that are not the trigger. + assert.strictEqual(SmartPicker.triggerStillThere(apiWith('bar'), '/pro'), false); + assert.strictEqual(SmartPicker.triggerStillThere(apiWith(''), '/pro'), false); + assert.strictEqual(SmartPicker.triggerStillThere(apiWith('prox'), '/pro'), false); + }); + + it('handles a bare "/" with nothing typed after it', function () { + // A caret sitting right after punctuation has no word before it. + assert.strictEqual(SmartPicker.triggerStillThere(apiWith(''), '/'), true); + assert.strictEqual(SmartPicker.triggerStillThere(apiWith('word'), '/'), false); + }); + + it('permits it where the editor cannot be asked', function () { + // asc_GetCurrentWord is exported by word/api.js only. Presentation + // and Spreadsheet keep the previous behaviour rather than lose the + // feature; they must not start refusing every deletion. + assert.strictEqual(SmartPicker.triggerStillThere({}, '/pro'), true); + assert.strictEqual(SmartPicker.triggerStillThere(null, '/pro'), true); + }); + + it('permits it when the probe throws', function () { + var api = {asc_GetCurrentWord: function () { throw new Error('boom'); }}; + assert.strictEqual(SmartPicker.triggerStillThere(api, '/pro'), true); + }); + }); + describe('createPending', function () { it('reports nothing outstanding before a request', function () { From f2519274d575c01beaf24b9393b4977e12e39c3b Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Tue, 18 Aug 2026 12:04:59 +0200 Subject: [PATCH 12/21] fix(smartpicker): repair the "/" caret session, from PR review Four defects in one file, all found in review of #130 and each reproduced in a running editor before it was touched. "/" only worked once per document. lastKey stands in for "the character before the caret" and nothing ever reset it, so typing "/" left it as "/" for good and slashCanTrigger refused every later trigger -- another cell, another text box, another paragraph -- until a space happened to be typed first. Anything that moves the caret elsewhere now forgets it. Undefined is the permissive value, which is the deliberate trade: this cannot read the document the way tiptap can, and a menu one Escape away beats a trigger that silently stops working. Clicking away did not end the session. sdkjs handles pointerdown on its canvas overlay and cancels it, so no compatibility mousedown is ever synthesised: measured in a running Writer, a click in the document area fires pointerdown and click on #id_viewer_overlay and no mousedown at all. The mousedown-only listener therefore never ran. The list was hidden by the editor's own hideAll(), so it looked dismissed, while Enter much later still opened the host's picker for whatever was left highlighted. Now bound to both. A keystroke between picking a provider and the host's modal taking focus cancelled the request. It is not proof the picker never opened -- the reply is still ours, and the character landed in the document behind the trigger. Within a hand-off grace such a key extends the text the reply has to delete instead, so "/f" plus a stray "y" no longer survives in front of the inserted link. triggerStillThere refused the commonest flow there is. It expected asc_GetCurrentWord(-1) to answer "" for a bare "/", on the grounds that punctuation is a word boundary. Measured against a running Writer it answers "/" -- the boundary rule holds only once a query follows -- so type "/", pick the first entry, and the "/" stayed in the document in front of the link. Alongside those: sanitizeIconUrl now rejects "/\host/path", which parses as the protocol-relative "//host/path"; the sdkjs element ids this feature reaches for are named once in one map instead of spelled out at each use; the option carrying a jQuery object is called `holder` rather than `holderEl`; and install() gathers the Gateway wiring the three editor controllers each carried a drifting copy of, so the next fix lands in all three at once. Tests cover all of it, including the two DOM-level regressions, which run in the browser harness only. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/lib/util/SmartPicker.js | 240 +++++++++++++++-- .../common/main/lib/util/SmartPicker.js | 252 +++++++++++++++++- 2 files changed, 470 insertions(+), 22 deletions(-) diff --git a/apps/common/main/lib/util/SmartPicker.js b/apps/common/main/lib/util/SmartPicker.js index 71a1c4aecf..749e3c5746 100644 --- a/apps/common/main/lib/util/SmartPicker.js +++ b/apps/common/main/lib/util/SmartPicker.js @@ -23,10 +23,15 @@ * - Accepting an entry replaces "/" plus the query with the picker's result, * the way tiptap's command() does deleteRange(range) before inserting. * - * Writer, Presentation and Spreadsheet share all of that. Only the insertion - * itself genuinely differs per editor, so only that stays in the controllers. + * Writer, Presentation and Spreadsheet share all of that, and share it through + * install() rather than by copy. Only what genuinely differs per editor stays in + * the controllers: how insertLink puts the reply into the document, and -- in + * the spreadsheet, which has no text caret until a cell is being edited -- where + * the menu is anchored. */ -define([], function () { 'use strict'; +define([ + 'common/main/lib/util/AssistantInsert' +], function () { 'use strict'; Common.Utils = Common.Utils || {}; @@ -37,6 +42,32 @@ define([], function () { 'use strict'; // Held down while typing a character; they must never be mistaken for one. var MODIFIER_KEYS = ['Shift', 'Control', 'Alt', 'AltGraph', 'Meta', 'CapsLock', 'Dead']; + // Keys that change what is before the caret without typing a character, so + // the last one typed stops describing it. Tab is here for the spreadsheet, + // where it commits the cell and moves to the next one. + var CARET_MOVED_KEYS = CARET_KEYS.concat(['Backspace', 'Tab']); + + // sdkjs element ids this feature reaches for, named once here rather than + // spelled out at each use. Shared with SmartPickerMenu, which anchors to + // two of them. + var IDS = Common.Utils.SmartPickerIds = { + // The scrollable document holder every editor renders into. Menus and + // popups in these editors are appended to it. + HOLDER: 'editor_sdk', + // The hidden input sdkjs types into. Matched as a prefix, not exactly: + // it also creates #area_id_main (the scrollable holder) and + // #area_id_parent, and which of them is the keydown target varies by + // editor and by edit state. + INPUT_AREA: 'area_id', + // The blinking caret itself, a 2x13px element the drawing document + // moves with the cursor (declared in each editor's api.js). + CARET: 'id_target_cursor', + // The IME wrapper, which sdkjs places below the caret. + IME_WRAPPER: 'area_id_parent' + }; + + var INPUT_AREA_RE = new RegExp(IDS.INPUT_AREA); + Common.Utils.SmartPicker = { /* @@ -63,6 +94,24 @@ define([], function () { 'use strict'; */ PENDING_TIMEOUT: 600000, + /* + * How long after picking a provider a keystroke still counts as part of + * the trigger rather than as proof that no reply is coming. + * + * Picking ends the caret session at once, but the host's modal only + * takes focus a moment later, so there is a real gap in which a + * keystroke still lands in the document, immediately behind the "/query" + * that is about to be deleted. Treating that as "the picker never + * opened" was wrong twice over -- the reply IS ours, and the extra + * character is part of what has to go -- and it showed as the trigger + * and the stray character surviving in front of the inserted link. + * + * Three seconds is far longer than the hand-off (a postMessage and a + * modal mount) and far shorter than a person reading the picker, so it + * separates the two cases without needing to know which one is running. + */ + HANDOFF_GRACE: 3000, + /** * Whether the trigger text is still sitting where it was typed. * @@ -85,10 +134,19 @@ define([], function () { 'use strict'; * is data loss that syncs to everyone. So this only permits the deletion * when the text before the caret still looks like what was typed. * - * It compares against the query rather than the whole trigger because the - * leading "/" is punctuation and therefore a word boundary: after "Hello - * /pro" the word part before the caret is "pro". A trigger of just "/" - * expects "", which is what a caret sitting right after punctuation gives. + * What "the word before the caret" means is sdkjs's answer, not a guess, + * and it is not the same for both shapes of trigger. Measured against a + * running Writer with the caret at the end: + * + * "Hello /pro" -> "pro" "/pro" -> "pro" + * "Hello /" -> "/" "" -> "" + * + * So "/" is a boundary once a query follows it, but on its own the + * punctuation run is itself the current word. Expecting "" for a bare + * trigger -- which is what the boundary rule alone suggests -- made this + * refuse every deletion for the commonest flow there is: type "/", pick + * the first entry, and the "/" stayed in the document in front of the + * link. * * Editors whose api does not expose asc_GetCurrentWord (Presentation and * Spreadsheet at the time of writing -- it is exported only in word/api.js) @@ -102,8 +160,9 @@ define([], function () { 'use strict'; triggerStillThere: function (api, replace) { if (!api || typeof api.asc_GetCurrentWord !== 'function') return true; - var expected = (replace || '').replace(/^\//, ''); - var actual; + var query = (replace || '').replace(/^\//, ''), + expected = query === '' ? (replace || '') : query, + actual; try { actual = api.asc_GetCurrentWord(-1); } catch (e) { @@ -155,7 +214,11 @@ define([], function () { 'use strict'; if (/["'<>\s]/.test(url)) return ''; if (/^https?:\/\//i.test(url)) return url; if (/^data:image\//i.test(url)) return url; - if (/^\/[^\/]/.test(url)) return url; // host-relative + // Host-relative. The second character must be neither "/" nor "\": + // browsers normalise backslashes to slashes when parsing a url, so + // "/\host/path" is the protocol-relative "//host/path" and would + // fetch from a foreign origin while reading as same-origin here. + if (/^\/[^\/\\]/.test(url)) return url; return ''; }, @@ -201,6 +264,34 @@ define([], function () { 'use strict'; return replace; }, + /** + * A key reached the editor while a request was outstanding. + * + * Within the hand-off grace the key was typed into the document + * behind the trigger, so it becomes part of what the reply has + * to delete. After it, the picker is not in front of the editor + * any more and no reply can still be coming, so the record goes. + * + * Backspace shortens the text again, but never past the "/" + * itself: once that is gone there is no trigger left to delete. + * + * @param {String} key the KeyboardEvent key + */ + activity: function (key) { + if (!this.isPending()) return; + if (Date.now() - _at <= Common.Utils.SmartPicker.HANDOFF_GRACE) { + if (typeof key === 'string' && key.length === 1) { + _replace += key; + return; + } + if (key === 'Backspace' && _replace.length > 1) { + _replace = _replace.slice(0, -1); + return; + } + } + this.clear(); + }, + clear: function () { _active = false; _at = 0; @@ -234,6 +325,29 @@ define([], function () { 'use strict'; query = null, // null while no menu session is running menu = function () { return Common.Views.SmartPickerMenu; }; + /* + * Forget which character was typed last. + * + * lastKey stands in for "the character before the caret", so it is + * only meaningful while the caret is where the typing left it. + * Anything that moves it elsewhere -- a click, an arrow key, a + * Backspace, Tab into the next cell -- makes it a statement about + * some other position, and undefined ("nothing typed yet") is the + * honest answer instead. + * + * Leaving it stale is what made the feature work exactly once per + * document: "/" sets lastKey to "/", and slashCanTrigger then + * rejects every later "/" -- in any cell, any text box, any + * paragraph -- until a space happened to be typed first. + * + * Undefined is the permissive value, so "/" typed after clicking + * into the middle of a word opens the menu where tiptap, which + * reads the real character out of the document, would not. That is + * the deliberate trade: this cannot see the document, and a menu + * one Escape away beats a trigger that silently stops working. + */ + var forgetPrevKey = function () { lastKey = undefined; }; + var closeSession = function () { if (query === null) return; query = null; @@ -243,7 +357,8 @@ define([], function () { 'use strict'; var startSession = function () { query = ''; menu().open({ - holderEl: options.getHolder(), + // jQuery-wrapped, as every getHolder() in these editors is. + holder: options.getHolder(), getAnchor: options.getAnchor, onPick: function (providerId) { // Replace the trigger and everything typed after it, @@ -329,7 +444,7 @@ define([], function () { 'use strict'; // the keydown target varies by editor and edit state. Anchoring // this to /^area_id$/ silently killed the trigger. var targetId = (e.target && e.target.id) || ''; - if (!/area_id/.test(targetId)) return; + if (!INPUT_AREA_RE.test(targetId)) return; if (query !== null) { if (handleSession(e)) { @@ -338,14 +453,19 @@ define([], function () { 'use strict'; return; } } else { - // Reaching the editor at all means the host's picker is not - // in front of it, so no reply can still be coming. - options.onActivity && options.onActivity(); + // Outside the hand-off gap, reaching the editor at all means + // the host's picker is not in front of it and no reply can + // still be coming. Inside it, the key was typed behind the + // trigger and has to be deleted with it -- which is why the + // key itself is passed on. See createPending().activity. + options.onActivity && options.onActivity(e.key); } var prevKey = lastKey; if (e.key.length === 1 || e.key === 'Enter') { lastKey = e.key; + } else if (CARET_MOVED_KEYS.indexOf(e.key) >= 0) { + forgetPrevKey(); } // Do not test altKey: browsers report AltGr as ctrl+alt, and on @@ -364,13 +484,95 @@ define([], function () { 'use strict'; document.addEventListener('keydown', handler, true); - // Clicking anywhere moves the caret out of the match -- except in - // the menu itself, where the click is how an entry gets picked. - document.addEventListener('mousedown', function (e) { + /* + * Clicking anywhere moves the caret out of the match -- except in + * the menu itself, where the click is how an entry gets picked. + * + * pointerdown, not just mousedown. sdkjs handles pointerdown on its + * canvas overlay (#id_viewer_overlay) and cancels it, so the browser + * never synthesises the compatibility mousedown at all: measured in a + * running Writer, one click in the document area fires pointerdown + * and click on the overlay and no mousedown whatsoever. A + * mousedown-only listener therefore never ran, and the session + * outlived the click -- the list was hidden by the editor's own + * hideAll(), so it looked dismissed, while Enter much later still + * opened the host's picker for whatever was left highlighted. + * + * Both are bound because pointer events are what the editor cancels, + * not what every path emits; closeSession() and forgetPrevKey() are + * idempotent, so handling the same gesture twice costs nothing. + */ + var onPointerDown = function (e) { + forgetPrevKey(); if (query === null) return; if (menu().ownsElement(e.target)) return; closeSession(); - }, true); + }; + document.addEventListener('pointerdown', onPointerDown, true); + document.addEventListener('mousedown', onPointerDown, true); + }, + + /** + * Wire one editor's Toolbar controller up to the Smart Picker. + * + * This was three copies, one per editor, and they had already drifted: + * different comment wording, one carrying a guard the others did not. + * Everything that is genuinely per-editor -- where the menu is anchored, + * and how insertLink puts the reply into the document -- stays with the + * editor; everything else is the same wiring three times, so it lives + * here and a fix now lands in all three at once. + * + * The controller keeps `_smartPickerAvailable`, because its own render + * path re-applies the button's visibility from it after a re-render. + * + * @param {Object} controller the editor's Toolbar controller + * @param {Object} options {getAnchor, onPick} per-editor hooks, optional + * @return {Object} the pending record insertLink consumes + */ + install: function (controller, options) { + options = options || {}; + var pending = this.createPending(); + + Common.Gateway.on('setsmartpickeravailable', function (available) { + controller._smartPickerAvailable = !!available; + var btn = controller.toolbar && controller.toolbar.btnSmartPicker; + btn && btn.setVisible(!!available); + }); + Common.Gateway.on('setsmartpickerproviders', function (data) { + // Pushed by the host rather than fetched by us: the list has to + // come from whichever page opens the picker, and only that page + // knows which providers it can actually open. + Common.Views.SmartPickerMenu.setProviders(data && data.providers); + }); + Common.Gateway.on('setsmartpickercancel', function () { + // The "/" and its query were typed by the user, so they stay -- + // as they do in Nextcloud's Text app when its picker is + // dismissed. Just restore focus. + pending.clear(); + Common.NotificationCenter.trigger('edit:complete'); + }); + Common.Gateway.on('insertassistantresult', function (data) { + // Sent when the user presses "Insert into document" in + // Nextcloud's own Assistant form. The host has already turned + // the model's markdown into HTML, so formatting survives. + Common.Utils.AssistantInsert.insert(controller.api, data || {}); + }); + + Common.Utils.SmartPicker.installTrigger({ + isAvailable: function () { return !!controller._smartPickerAvailable; }, + onActivity: function (key) { pending.activity(key); }, + getHolder: function () { return $('#' + IDS.HOLDER); }, + getAnchor: options.getAnchor, + onPick: function (providerId, replace) { + // replace is "/" plus whatever was typed after it; the reply + // deletes exactly that before inserting the link. + pending.begin(replace); + options.onPick && options.onPick(replace); + Common.Gateway.requestSmartPicker('', 'toolbar', providerId); + } + }); + + return pending; } }; diff --git a/test/unit-tests/common/main/lib/util/SmartPicker.js b/test/unit-tests/common/main/lib/util/SmartPicker.js index 5b6be2293d..7be4b5ecf8 100644 --- a/test/unit-tests/common/main/lib/util/SmartPicker.js +++ b/test/unit-tests/common/main/lib/util/SmartPicker.js @@ -117,6 +117,15 @@ assert.strictEqual(clean('//evil.example/i.png'), ''); }); + it('drops a backslash disguised as a host-relative path', function () { + // Browsers normalise "\" to "/" when parsing a url, so + // "/\evil.example/i.png" is the protocol-relative + // "//evil.example/i.png" -- a foreign origin behind a string + // that reads as same-origin. + assert.strictEqual(clean('/\\evil.example/i.png'), ''); + assert.strictEqual(clean('\\\\evil.example/i.png'), ''); + }); + it('drops non-strings and empty values', function () { assert.strictEqual(clean(undefined), ''); assert.strictEqual(clean(null), ''); @@ -128,7 +137,10 @@ describe('triggerStillThere', function () { // The word part before the caret, which is what asc_GetCurrentWord(-1) - // returns. "/" is punctuation, so it is not part of that word. + // returns. The answers below are the ones measured against a running + // Writer, not what the punctuation rule alone would suggest: with a + // query after it "/" is a boundary ("Hello /pro" -> "pro"), but a + // bare trigger answers with the punctuation itself ("Hello /" -> "/"). var apiWith = function (wordBeforeCaret) { return {asc_GetCurrentWord: function () { return wordBeforeCaret; }}; }; @@ -146,9 +158,13 @@ }); it('handles a bare "/" with nothing typed after it', function () { - // A caret sitting right after punctuation has no word before it. - assert.strictEqual(SmartPicker.triggerStillThere(apiWith(''), '/'), true); + // The commonest flow of all: type "/", pick the first entry. + // Expecting '' here instead is what left the "/" sitting in + // front of every link inserted that way. + assert.strictEqual(SmartPicker.triggerStillThere(apiWith('/'), '/'), true); assert.strictEqual(SmartPicker.triggerStillThere(apiWith('word'), '/'), false); + // An empty answer means the caret is not after a "/" any more. + assert.strictEqual(SmartPicker.triggerStillThere(apiWith(''), '/'), false); }); it('permits it where the editor cannot be asked', function () { @@ -225,5 +241,235 @@ } }); }); + + describe('createPending activity', function () { + + // Picking a provider ends the caret session at once, but the host's + // modal only takes focus a moment later. Keys landing in that gap go + // into the document behind the trigger, and used to cancel the + // request instead -- which left "/query" and the stray character + // sitting in front of the link that then arrived. + var atTime = function (body) { + var realNow = Date.now, now = 1000000; + Date.now = function () { return now; }; + try { + body(function (ms) { now += ms; }); + } finally { + Date.now = realNow; + } + }; + + it('takes a key typed during the hand-off as part of the trigger', function () { + atTime(function (advance) { + var pending = SmartPicker.createPending(); + pending.begin('/f'); + advance(200); + pending.activity('y'); + assert.strictEqual(pending.isPending(), true); + assert.strictEqual(pending.consume(), '/fy'); + }); + }); + + it('lets Backspace take it away again, but not the "/" itself', function () { + atTime(function () { + var pending = SmartPicker.createPending(); + pending.begin('/f'); + pending.activity('y'); + pending.activity('Backspace'); + assert.strictEqual(pending.consume(), '/f'); + + pending.begin('/'); + pending.activity('Backspace'); + // Nothing of the trigger is left to shorten, so the request + // is dropped rather than aimed at a "/" that has gone. + assert.strictEqual(pending.isPending(), false); + }); + }); + + it('drops the request on a key that moves the caret', function () { + atTime(function () { + var pending = SmartPicker.createPending(); + pending.begin('/f'); + pending.activity('ArrowLeft'); + assert.strictEqual(pending.consume(), null); + }); + }); + + it('drops the request once the hand-off is long over', function () { + atTime(function (advance) { + var pending = SmartPicker.createPending(); + pending.begin('/f'); + advance(SmartPicker.HANDOFF_GRACE + 1); + // Typing in the editor now means the picker is not in front + // of it, so no reply is coming and the record must not sit + // there waiting to delete something. + pending.activity('y'); + assert.strictEqual(pending.isPending(), false); + }); + }); + + it('ignores activity when nothing is outstanding', function () { + var pending = SmartPicker.createPending(); + pending.activity('y'); + assert.strictEqual(pending.isPending(), false); + assert.strictEqual(pending.consume(), null); + }); + }); + + // installTrigger listens on the real document and drives the real menu, + // so these run in the browser harness only; `node --test` covers the + // pure functions above. + if (typeof document !== 'undefined' && typeof KeyboardEvent === 'function') { + describe('installTrigger', function () { + + var area, menu, opened, picked, savedMenu, installed; + + var press = function (key) { + area.dispatchEvent(new KeyboardEvent('keydown', { + key: key, bubbles: true, cancelable: true + })); + }; + + // A real pointerdown, which is what the editor's canvas overlay + // emits -- and cancels, so no mousedown ever follows it. + var pointerDown = function (target) { + (target || area).dispatchEvent(new MouseEvent('pointerdown', { + bubbles: true, cancelable: true + })); + }; + + // startSession runs from _.defer, which is _.delay(fn, 1) -- + // a timer, not a microtask, so waiting has to outlast it. + var afterDefer = function (body) { setTimeout(body, 20); }; + + before(function () { + area = document.createElement('div'); + area.id = 'area_id'; + document.body.appendChild(area); + + savedMenu = Common.Views && Common.Views.SmartPickerMenu; + Common.Views = Common.Views || {}; + menu = Common.Views.SmartPickerMenu = { + _open: false, + open: function (options) { this._open = true; opened++; this._onPick = options.onPick; }, + close: function () { this._open = false; }, + isOpen: function () { return this._open; }, + filter: function () {}, + moveSelection: function () {}, + pickSelected: function () { this._onPick('files'); return true; }, + ownsElement: function () { return false; } + }; + + // One installation for the whole suite: it binds to the + // document for good, exactly as an editor does. + installed = {available: true}; + SmartPicker.installTrigger({ + isAvailable: function () { return installed.available; }, + onActivity: function () {}, + getHolder: function () { return $(document.body); }, + onPick: function (providerId, replace) { picked = {providerId: providerId, replace: replace}; } + }); + }); + + after(function () { + area.parentNode.removeChild(area); + Common.Views.SmartPickerMenu = savedMenu; + }); + + beforeEach(function () { + opened = 0; + picked = null; + menu._open = false; + }); + + it('opens on "/" typed after a space', function (done) { + press(' '); + press('/'); + afterDefer(function () { + assert.strictEqual(opened, 1); + press('Escape'); + done(); + }); + }); + + it('opens again in a new place after a click', function (done) { + // The regression: "/" left lastKey as "/" for good, so + // slashCanTrigger rejected every later trigger and the + // feature worked exactly once per document. A click moves + // the caret somewhere this cannot see, which is the same + // state as "nothing typed yet". + press(' '); + press('/'); + afterDefer(function () { + press('Escape'); + pointerDown(); + press('/'); + afterDefer(function () { + assert.strictEqual(opened, 2, '"/" must trigger again after a click'); + press('Escape'); + done(); + }); + }); + }); + + it('opens again after an arrow key has moved the caret', function (done) { + press(' '); + press('/'); + afterDefer(function () { + press('Escape'); + press('ArrowRight'); + press('/'); + afterDefer(function () { + assert.strictEqual(opened, 2); + press('Escape'); + done(); + }); + }); + }); + + it('still refuses a second "/" typed straight after the first', function (done) { + // "//" is literal in tiptap too: the character before the + // trigger is the old one, not whitespace. + press(' '); + press('/'); + afterDefer(function () { + press('Escape'); + press('/'); + afterDefer(function () { + assert.strictEqual(opened, 1); + done(); + }); + }); + }); + + it('ends the session on a pointerdown outside the menu', function (done) { + // mousedown never arrives: sdkjs cancels the pointerdown on + // its canvas overlay, so the browser synthesises no + // compatibility mouse events at all. A session left running + // here looked dismissed but reopened the host picker on the + // next Enter. + press(' '); + press('/'); + afterDefer(function () { + pointerDown(); + assert.strictEqual(menu.isOpen(), false, 'the menu must be disposed'); + press('Enter'); + assert.strictEqual(picked, null, 'Enter must not pick after the click'); + done(); + }); + }); + + it('does not open when the host says the picker is unavailable', function (done) { + installed.available = false; + press(' '); + press('/'); + afterDefer(function () { + installed.available = true; + assert.strictEqual(opened, 0); + done(); + }); + }); + }); + } }); })); From 5fed4b734ebdff4f7214f92072930f5ac28fac22 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Tue, 18 Aug 2026 12:04:59 +0200 Subject: [PATCH 13/21] fix(smartpicker): follow the caret when the window is resized The comment on _position() said the container has to be re-aligned "again after any resize", and nothing ever listened for one: the menu stayed where it was opened while the document reflowed underneath it. Re-reading the anchor rather than re-clamping the old point is what a resize actually calls for -- the caret moves, and the cell the spreadsheet anchors to moves with it. Verified in a running Writer: shrinking the viewport moved the caret from x=431 to x=272 and the menu followed, where before it stayed at its original x. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/lib/view/SmartPickerMenu.js | 41 ++++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/apps/common/main/lib/view/SmartPickerMenu.js b/apps/common/main/lib/view/SmartPickerMenu.js index fdcc5fa265..210fde9684 100644 --- a/apps/common/main/lib/view/SmartPickerMenu.js +++ b/apps/common/main/lib/view/SmartPickerMenu.js @@ -37,7 +37,10 @@ define([ Common.Views = Common.Views || {}; Common.Views.SmartPickerMenu = _.extend(new(function() { - var CONTAINER_ID = 'menu-container-smartpicker'; + var CONTAINER_ID = 'menu-container-smartpicker', + // Namespaced so off() takes back exactly this menu's handler and + // nothing else bound to window resize. + RESIZE_EVENT = 'resize.smartpickermenu'; var _menu, // Common.UI.Menu, or undefined when closed _providers, // pushed in by the host @@ -46,6 +49,7 @@ define([ _emptyRow, // the "no suggestion found"
  • _selected = -1, // index into _visible _onPick, + _getAnchor, // editor-supplied anchor, re-read on resize _point; // anchor captured when the menu opened /** @@ -63,7 +67,7 @@ define([ * @return {Array|null} [left, top] in viewport coordinates */ var _caretPoint = function() { - var el = document.getElementById('id_target_cursor'); + var el = document.getElementById(Common.Utils.SmartPickerIds.CARET); if (el) { var r = el.getBoundingClientRect(); // Visible caret: place the menu just under it. Height is not @@ -76,7 +80,7 @@ define([ } // Fallback: the IME wrapper. Already offset past the caret bottom by // sdkjs, so take its top as-is. - var alt = document.getElementById('area_id_parent'); + var alt = document.getElementById(Common.Utils.SmartPickerIds.IME_WRAPPER); if (alt && alt.getBoundingClientRect) { var ar = alt.getBoundingClientRect(); if (ar && (ar.left || ar.top)) { @@ -169,15 +173,35 @@ define([ _menu.remove(); _menu = undefined; } + $(window).off(RESIZE_EVENT); $('#' + CONTAINER_ID).remove(); _entries = []; _visible = []; _emptyRow = undefined; _selected = -1; _onPick = undefined; + _getAnchor = undefined; _point = undefined; }; + /* + * Follow the caret after the viewport changes. + * + * _position() alone would only re-clamp the menu against the new + * viewport, which is not what a resize does to the thing it is anchored + * to: reflowing the document moves the caret, and the cell the + * spreadsheet anchors to moves with it. So the anchor is re-read, not + * remembered -- the same call _open() makes -- and only kept if it still + * resolves, since a caret that has gone away should leave the menu where + * it is rather than send it to the corner. + */ + var _reanchor = function() { + if (!_menu) return; + var point = (_getAnchor && _getAnchor()) || _caretPoint(); + if (point) _point = point; + _position(); + }; + /* Move the highlight, without moving focus away from the document. */ var _highlight = function(index) { _entries.forEach(function(entry) { @@ -227,7 +251,8 @@ define([ }; /** - * @param {Object} options {holderEl, getAnchor, onPick} + * @param {Object} options {holder, getAnchor, onPick}; holder is the + * jQuery-wrapped element the container is appended to */ var _open = function(options) { _dispose(); @@ -240,7 +265,7 @@ define([ // position:fixed keeps the container independent of whether the // document holder is a positioned ancestor -- it is not. - var holder = $(options.holderEl), + var holder = $(options.holder), container = $('
    ' + '
    '); holder.append(container); @@ -258,7 +283,8 @@ define([ // An editor that knows better says so: the spreadsheet anchors to // the active cell, because it has no text caret unless a cell is // being edited inline. - _point = (options.getAnchor && options.getAnchor()) || _caretPoint(); + _getAnchor = options.getAnchor; + _point = (_getAnchor && _getAnchor()) || _caretPoint(); if (!_point) { // No caret anchor: fall back to the holder's top-left. var hr = holder[0] ? holder[0].getBoundingClientRect() : {left: 40, top: 60}; @@ -269,6 +295,7 @@ define([ // wherever the container currently sits. _position(); _menu.show(); + $(window).on(RESIZE_EVENT, _reanchor); // Deliberately no focus() here: the user is still typing into the // document, and taking focus would send those keys to the list. _applyFilter(''); @@ -296,7 +323,7 @@ define([ * Never fails silently: this runs from a keystroke, so a thrown * error would look to the user like "/" simply does nothing. * - * @param {Object} options {holderEl, getAnchor, onPick} + * @param {Object} options {holder, getAnchor, onPick} */ open: function(options) { try { From 1dd04153817a900fcc5e31a6e0f75c0ce992f1cf Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Tue, 18 Aug 2026 12:04:59 +0200 Subject: [PATCH 14/21] fix(assistant): stop losing a result the editor was too busy to take The retry loop waited two seconds for sdkjs's PasteHtml re-entrancy guard to clear and then called PasteHtml anyway -- the one call its own comment says that guard drops without a word. On a slow machine the Assistant's answer disappeared and nothing said so. Falls back to PasteText, which is not behind that guard and keeps the content at the cost of the formatting, and only warns when there is no text to fall back on. The guard element id and the retry budget are named rather than inline. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/lib/util/AssistantInsert.js | 37 ++++++++++++++++---- apps/documenteditor/main/locale/en.json | 1 + apps/presentationeditor/main/locale/en.json | 1 + 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/apps/common/main/lib/util/AssistantInsert.js b/apps/common/main/lib/util/AssistantInsert.js index 35db8c906f..a7d6d88568 100644 --- a/apps/common/main/lib/util/AssistantInsert.js +++ b/apps/common/main/lib/util/AssistantInsert.js @@ -14,7 +14,16 @@ define([], function () { 'use strict'; Common.Utils = Common.Utils || {}; - Common.Utils.AssistantInsert = { + // sdkjs parks this element on the page for the duration of a PasteHtml and + // refuses a second one while it is there (api_plugins.js). + var PASTE_GUARD_ID = 'pmpastehtml'; + + // 100 ms x 20 = 2 s. A paste of a few paragraphs is done in a fraction of + // that; anything still running after it is not about to finish either. + var RETRY_INTERVAL = 100, + MAX_ATTEMPTS = 20; + + Common.Utils.AssistantInsert = _.extend({ /** * @param {Object} api the editor api @@ -32,13 +41,26 @@ define([], function () { 'use strict'; // PasteHtml is re-entrancy guarded on this element, so a second // insertion while one is still running is dropped. Wait it out // rather than losing the result. - if (document.getElementById('pmpastehtml')) { - if (attempt < 20) { + if (document.getElementById(PASTE_GUARD_ID)) { + if (attempt < MAX_ATTEMPTS) { setTimeout(function() { Common.Utils.AssistantInsert.insert(api, result, attempt + 1); - }, 100); + }, RETRY_INTERVAL); + return; + } + // Still guarded. Calling PasteHtml anyway is not a fallback: + // the guard drops it without a word, and the answer the user + // waited for would simply never appear in the document. + // Plain text loses the formatting but keeps the content, so + // try that; only when there is none is there nothing left to + // do but say so. + if (text && typeof api['pluginMethod_PasteText'] === 'function') { + api['pluginMethod_PasteText'](text); + Common.NotificationCenter.trigger('edit:complete'); return; } + Common.UI.warning({msg: Common.Utils.AssistantInsert.txtInsertFailed}); + return; } api['pluginMethod_PasteHtml'](html); } else if (typeof api['pluginMethod_PasteText'] === 'function') { @@ -46,8 +68,11 @@ define([], function () { 'use strict'; } Common.NotificationCenter.trigger('edit:complete'); - } - }; + }, + + txtInsertFailed: 'The editor is busy, so the Assistant result could not be inserted. Try inserting it again.' + + }, Common.Utils.AssistantInsert || {}); return Common.Utils.AssistantInsert; }); diff --git a/apps/documenteditor/main/locale/en.json b/apps/documenteditor/main/locale/en.json index 2c1dd94852..83194976a8 100644 --- a/apps/documenteditor/main/locale/en.json +++ b/apps/documenteditor/main/locale/en.json @@ -696,6 +696,7 @@ "Common.UI.Window.textInformation": "Information", "Common.UI.Window.textWarning": "Warning", "Common.UI.Window.yesButtonText": "Yes", + "Common.Utils.AssistantInsert.txtInsertFailed": "The editor is busy, so the Assistant result could not be inserted. Try inserting it again.", "Common.Utils.Metric.txtCm": "cm", "Common.Utils.Metric.txtPt": "pt", "Common.Utils.String.textAlt": "Alt", diff --git a/apps/presentationeditor/main/locale/en.json b/apps/presentationeditor/main/locale/en.json index 74011c6d85..facb2b48b5 100644 --- a/apps/presentationeditor/main/locale/en.json +++ b/apps/presentationeditor/main/locale/en.json @@ -783,6 +783,7 @@ "Common.UI.Window.textInformation": "Information", "Common.UI.Window.textWarning": "Warning", "Common.UI.Window.yesButtonText": "Yes", + "Common.Utils.AssistantInsert.txtInsertFailed": "The editor is busy, so the Assistant result could not be inserted. Try inserting it again.", "Common.Utils.Metric.txtCm": "cm", "Common.Utils.Metric.txtPt": "pt", "Common.Utils.String.textAlt": "Alt", From 75e66e013a7cb59fa5f1f740ab0ad6cd5169d0d9 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Tue, 18 Aug 2026 12:04:59 +0200 Subject: [PATCH 15/21] fix(smartpicker): aim the reply at the request that asked for it Three things the three controllers each needed, and now share. The toolbar button registered nothing with the pending record and did not clear it either, so a request the host never answered nor cancelled -- its modal closed some way that told us nothing -- was still sitting there when the button's own reply arrived, and that insertion would delete a "/query" typed minutes ago somewhere else. It clears the record first. In the spreadsheet the deletion is aimed blind: triggerStillThere needs asc_GetCurrentWord, which is exported only in word/api.js, so it always answers "cannot tell" there. The cell the trigger was typed into is recorded with the request and compared when the reply lands; a selection reached by scrolling or by a co-author's change is not where the "/" was, so nothing of ours is deleted there. The Gateway wiring itself moves into Common.Utils.SmartPicker.install: it was three copies that had already drifted in comment wording and in which guards each carried. Only the anchor, the recorded cell and insertLink stay per editor. Also: txtCellNotEmpty comes from the locale file with no hardcoded English beside it, as every other string these controllers show does; and the edit-mode branch of insertLink now says why it deliberately has no empty-cell guard -- it writes at the cursor the user put there, where the branch below appends to a value it never read. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- .../main/app/controller/Toolbar.js | 44 ++------ .../main/app/controller/Toolbar.js | 45 ++------ .../main/app/controller/Toolbar.js | 101 +++++++++++------- apps/spreadsheeteditor/main/locale/en.json | 1 + 4 files changed, 81 insertions(+), 110 deletions(-) diff --git a/apps/documenteditor/main/app/controller/Toolbar.js b/apps/documenteditor/main/app/controller/Toolbar.js index 63236288bd..e4a529adf0 100644 --- a/apps/documenteditor/main/app/controller/Toolbar.js +++ b/apps/documenteditor/main/app/controller/Toolbar.js @@ -34,7 +34,6 @@ define([ 'core', 'common/main/lib/component/Window', - 'common/main/lib/util/AssistantInsert', 'common/main/lib/util/SmartPicker', 'common/main/lib/view/SmartPickerMenu', 'documenteditor/main/app/view/Toolbar', @@ -256,40 +255,9 @@ define([ Common.NotificationCenter.on('app:ready', me.onAppReady.bind(me)); Common.NotificationCenter.on('app:face', me.onAppShowed.bind(me)); - Common.Gateway.on('setsmartpickeravailable', function(available) { - me._smartPickerAvailable = !!available; - me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); - }); - Common.Gateway.on('insertassistantresult', function(data) { - // Sent when the user presses "Insert into document" in Nextcloud's - // own Assistant form. The host has already turned the model's - // markdown into HTML, so formatting survives. - Common.Utils.AssistantInsert.insert(me.api, data || {}); - }); - Common.Gateway.on('setsmartpickerproviders', function(data) { - // Pushed by the host rather than fetched by us: the list has to - // come from whichever page opens the picker, and only that page - // knows which providers it can actually open. - Common.Views.SmartPickerMenu.setProviders(data && data.providers); - }); - Common.Gateway.on('setsmartpickercancel', function() { - // The "/" was typed by the user, so it stays -- as it does in - // Nextcloud's own editors when their picker is dismissed. - me._smartPicker.clear(); - Common.NotificationCenter.trigger('edit:complete'); - }); - me._smartPicker = Common.Utils.SmartPicker.createPending(); - Common.Utils.SmartPicker.installTrigger({ - isAvailable: function() { return !!me._smartPickerAvailable; }, - onActivity: function() { me._smartPicker.clear(); }, - getHolder: function() { return $('#editor_sdk'); }, - onPick: function(providerId, replace) { - // replace is "/" plus whatever was typed after it; the reply - // deletes exactly that before inserting the link. - me._smartPicker.begin(replace); - Common.Gateway.requestSmartPicker('', 'toolbar', providerId); - } - }); + // Gateway wiring, the "/" trigger and the pending record are the + // same in all three editors; only insertLink below is not. + me._smartPicker = Common.Utils.SmartPicker.install(me); }, setMode: function(mode) { @@ -3522,6 +3490,12 @@ define([ onSmartPickerClick: function() { if (!this.api) return; + // Whatever the caret menu may still be waiting for, this reply + // will not be it. Dropping the record here is what keeps a request + // the host never answered or cancelled -- its modal closed some way + // that told us nothing -- from making this insertion delete a + // "/query" the user typed minutes ago somewhere else. + this._smartPicker.clear(); // Open Nextcloud's own Smart Picker, with no provider preselected so it // shows its provider list. Deliberately not our caret menu: that exists // to keep the "/" flow inside the editor, whereas this button is the diff --git a/apps/presentationeditor/main/app/controller/Toolbar.js b/apps/presentationeditor/main/app/controller/Toolbar.js index 5f81b98d19..060dacc466 100644 --- a/apps/presentationeditor/main/app/controller/Toolbar.js +++ b/apps/presentationeditor/main/app/controller/Toolbar.js @@ -35,7 +35,6 @@ define([ 'core', 'common/main/lib/component/Window', - 'common/main/lib/util/AssistantInsert', 'common/main/lib/util/SmartPicker', 'common/main/lib/view/SmartPickerMenu', 'presentationeditor/main/app/collection/SlideThemes', @@ -283,41 +282,9 @@ define([ PE.getCollection('SlideLayouts').bind({ reset: me.onResetSlides.bind(this) }); - Common.Gateway.on('setsmartpickeravailable', function(available) { - me._smartPickerAvailable = !!available; - me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); - }); - Common.Gateway.on('insertassistantresult', function(data) { - // Sent when the user presses "Insert into document" in Nextcloud's - // own Assistant form. The host has already turned the model's - // markdown into HTML, so formatting survives. - Common.Utils.AssistantInsert.insert(me.api, data || {}); - }); - Common.Gateway.on('setsmartpickerproviders', function(data) { - // Pushed by the host rather than fetched by us: the list has to - // come from whichever page opens the picker, and only that page - // knows which providers it can actually open. - Common.Views.SmartPickerMenu.setProviders(data && data.providers); - }); - Common.Gateway.on('setsmartpickercancel', function() { - // The "/" and its query were typed by the user, so they stay -- - // as they do in Nextcloud's Text app when its picker is - // dismissed. - me._smartPicker.clear(); - Common.NotificationCenter.trigger('edit:complete'); - }); - me._smartPicker = Common.Utils.SmartPicker.createPending(); - Common.Utils.SmartPicker.installTrigger({ - isAvailable: function() { return !!me._smartPickerAvailable; }, - onActivity: function() { me._smartPicker.clear(); }, - getHolder: function() { return $('#editor_sdk'); }, - onPick: function(providerId, replace) { - // replace is "/" plus whatever was typed after it; the reply - // deletes exactly that before inserting the link. - me._smartPicker.begin(replace); - Common.Gateway.requestSmartPicker('', 'toolbar', providerId); - } - }); + // Gateway wiring, the "/" trigger and the pending record are the + // same in all three editors; only insertLink below is not. + me._smartPicker = Common.Utils.SmartPicker.install(me); }, setMode: function(mode) { @@ -1886,6 +1853,12 @@ define([ onSmartPickerClick: function() { if (!this.api) return; + // Whatever the caret menu may still be waiting for, this reply + // will not be it. Dropping the record here is what keeps a request + // the host never answered or cancelled -- its modal closed some way + // that told us nothing -- from making this insertion delete a + // "/query" the user typed minutes ago somewhere else. + this._smartPicker.clear(); // Open Nextcloud's own Smart Picker, with no provider preselected so it // shows its provider list. Deliberately not our caret menu: that exists // to keep the "/" flow inside the editor, whereas this button is the diff --git a/apps/spreadsheeteditor/main/app/controller/Toolbar.js b/apps/spreadsheeteditor/main/app/controller/Toolbar.js index f172642635..f60403a071 100644 --- a/apps/spreadsheeteditor/main/app/controller/Toolbar.js +++ b/apps/spreadsheeteditor/main/app/controller/Toolbar.js @@ -32,7 +32,6 @@ define([ 'core', 'common/main/lib/component/Window', - 'common/main/lib/util/AssistantInsert', 'common/main/lib/util/SmartPicker', 'common/main/lib/view/SmartPickerMenu', 'common/main/lib/view/SearchBar', @@ -267,34 +266,10 @@ define([ Common.NotificationCenter.on('app:ready', this.onAppReady.bind(this)); Common.NotificationCenter.on('app:face', this.onAppShowed.bind(this)); - Common.Gateway.on('setsmartpickeravailable', function(available) { - me._smartPickerAvailable = !!available; - me.toolbar.btnSmartPicker && me.toolbar.btnSmartPicker.setVisible(!!available); - }); - Common.Gateway.on('insertassistantresult', function(data) { - // Sent when the user presses "Insert into document" in Nextcloud's - // own Assistant form. The host has already turned the model's - // markdown into HTML, so formatting survives. - Common.Utils.AssistantInsert.insert(me.api, data || {}); - }); - Common.Gateway.on('setsmartpickerproviders', function(data) { - // Pushed by the host rather than fetched by us: the list has to - // come from whichever page opens the picker, and only that page - // knows which providers it can actually open. - Common.Views.SmartPickerMenu.setProviders(data && data.providers); - }); - Common.Gateway.on('setsmartpickercancel', function() { - // The "/" and its query were typed by the user, so they stay -- - // as they do in Nextcloud's Text app when its picker is - // dismissed. Just restore focus. - me._smartPicker.clear(); - Common.NotificationCenter.trigger('edit:complete'); - }); - me._smartPicker = Common.Utils.SmartPicker.createPending(); - Common.Utils.SmartPicker.installTrigger({ - isAvailable: function() { return !!me._smartPickerAvailable; }, - onActivity: function() { me._smartPicker.clear(); }, - getHolder: function() { return $('#editor_sdk'); }, + // Gateway wiring, the "/" trigger and the pending record are the + // same in all three editors; the anchor, the cell the request came + // from and insertLink below are not. + me._smartPicker = Common.Utils.SmartPicker.install(me, { // Anchor to the active cell. There is no text caret here until a // cell is being edited inline, so the caret element the other // editors use sits wherever it was last left. This is how every @@ -318,11 +293,12 @@ define([ Math.round(rect.top + y + coord.asc_getHeight()) ]; }, - onPick: function(providerId, replace) { - // replace is "/" plus whatever was typed after it; the reply - // deletes exactly that before inserting the link. - me._smartPicker.begin(replace); - Common.Gateway.requestSmartPicker('', 'toolbar', providerId); + onPick: function() { + // Which cell the trigger was typed into. The other two + // editors get this guard from triggerStillThere, which needs + // asc_GetCurrentWord -- exported only in word/api.js -- so + // here the deletion is otherwise aimed blind. See insertLink. + me._smartPickerCell = me.getSmartPickerCell(); } }); }, @@ -1130,6 +1106,12 @@ define([ onSmartPickerClick: function() { if (!this.api) return; + // Whatever the caret menu may still be waiting for, this reply + // will not be it. Dropping the record here is what keeps a request + // the host never answered or cancelled -- its modal closed some way + // that told us nothing -- from making this insertion delete a + // "/query" the user typed minutes ago somewhere else. + this._smartPicker.clear(); // Open Nextcloud's own Smart Picker, with no provider preselected so it // shows its provider list. Deliberately not our caret menu: that exists // to keep the "/" flow inside the editor, whereas this button is the @@ -1451,6 +1433,26 @@ define([ Common.NotificationCenter.trigger('storage:image-insert', data); }, + /** + * Where the selection is, as one comparable string. + * + * Absolute (referenceType.A) so a relative reference cannot make two + * different cells look alike, and prefixed with the sheet, because + * $B$3 on another sheet is another cell. + * + * @return {String} e.g. "0!$B$3", or '' when the api cannot say + */ + getSmartPickerCell: function() { + if (!this.api || typeof this.api.asc_getActiveRangeStr !== 'function') return ''; + try { + return this.api.asc_getActiveWorksheetIndex() + '!' + + this.api.asc_getActiveRangeStr(Asc.referenceType.A); + } catch (e) { + // Never let a probe stop an insertion. + return ''; + } + }, + insertLink: function(data) { // gateway if (!this.api) return; // null when this reply is not the answer to our request, in which @@ -1461,6 +1463,18 @@ define([ if (replace && !Common.Utils.SmartPicker.triggerStillThere(this.api, replace)) { replace = null; } + // And only if the selection has not moved since the request. Both + // insertion paths below act on wherever the selection is now, and + // triggerStillThere cannot help here: asc_GetCurrentWord is exported + // only in word/api.js, so it always answers "cannot tell" in the + // spreadsheet and the deletion would go in blind. A cell reached by + // scrolling or by a co-author's change is not where the "/" was + // typed, so there is nothing of ours to delete there. + if (replace && this._smartPickerCell && + this._smartPickerCell !== this.getSmartPickerCell()) { + replace = null; + } + this._smartPickerCell = ''; if (replace !== null) { // Insert the link as plain TEXT (a cell hyperlink is whole-cell // and would re-link/replace the cell). NOTE: use isCellEdited, @@ -1475,7 +1489,17 @@ define([ // first or pluginMethod's addText won't land. (text-input // path never adds a "+" the way asc_insertInCell while // editing would.) - var areaEl = document.getElementById('area_id'); + // + // Deliberately no "is the cell empty" guard here, unlike the + // branch below. That guard exists because that branch + // *appends* to a value it did not read from the caret, so a + // formula or a number silently becomes a broken string. This + // one writes at the cursor the user put there by typing "/", + // between exactly the characters they chose -- the same thing + // typing any other character would do. Refusing it would + // reject the feature's normal case, "Hello /f", not protect + // anything. + var areaEl = document.getElementById(Common.Utils.SmartPickerIds.INPUT_AREA); if (areaEl && areaEl.focus) { try { areaEl.focus(); } catch (e) {} } if (typeof this.api['pluginMethod_InputText'] === 'function') { this.api['pluginMethod_InputText'](data, replace); @@ -1504,10 +1528,9 @@ define([ // where the branch above inserts at the cursor. if (cur !== '') { Common.NotificationCenter.trigger('edit:complete'); - Common.UI.warning({ - msg: this.txtCellNotEmpty - || 'This cell is not empty. Insert the link into an empty cell, or double-click the cell to place it inside.' - }); + // From the locale file, as every other string this + // controller shows is (see txtInvalidRange). + Common.UI.warning({msg: this.txtCellNotEmpty}); return; } this.api.asc_insertInCell(data, Asc.c_oAscPopUpSelectorType.None); diff --git a/apps/spreadsheeteditor/main/locale/en.json b/apps/spreadsheeteditor/main/locale/en.json index f038637cef..007c9ffd5f 100644 --- a/apps/spreadsheeteditor/main/locale/en.json +++ b/apps/spreadsheeteditor/main/locale/en.json @@ -730,6 +730,7 @@ "Common.UI.Window.textInformation": "Information", "Common.UI.Window.textWarning": "Warning", "Common.UI.Window.yesButtonText": "Yes", + "Common.Utils.AssistantInsert.txtInsertFailed": "The editor is busy, so the Assistant result could not be inserted. Try inserting it again.", "Common.Utils.Metric.txtCm": "cm", "Common.Utils.Metric.txtPt": "pt", "Common.Utils.String.textAlt": "Alt", From 0b9fd09257010491ae0bf24185c4b3366ca46e1f Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Tue, 18 Aug 2026 12:04:59 +0200 Subject: [PATCH 16/21] fix(icons): give "Ask Nextcloud Assistant" its own icon back This branch needed a plus for the new "Add from Nextcloud" button and took it by repointing btn-nc-assistant at it and deleting the sparkle, which also changed the icon of the context-menu item for the Assistant -- a different, untouched feature -- in all three editors. btn-nc-assistant is restored and the menu item points at it again; btn-big-nc-add is the button's own icon. The 24px btn-nc-add had no user left once the menu item stopped borrowing it, so it goes rather than sit in every sprite unused. Sprites regenerated with build/scripts/deploy-sprites.js, which makes the diff against main purely additive. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/common/main/resources/img/toolbar/icons.svg | 6 ++++-- .../main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg | 3 --- .../main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg | 5 +++++ apps/documenteditor/main/app/view/DocumentHolderExt.js | 2 +- apps/documenteditor/main/resources/img/toolbar/icons.svg | 6 ++++-- apps/pdfeditor/main/resources/img/toolbar/icons.svg | 6 ++++-- apps/presentationeditor/main/app/view/DocumentHolderExt.js | 2 +- .../presentationeditor/main/resources/img/toolbar/icons.svg | 6 ++++-- apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js | 2 +- apps/spreadsheeteditor/main/resources/img/toolbar/icons.svg | 6 ++++-- apps/visioeditor/main/resources/img/toolbar/icons.svg | 6 ++++-- 11 files changed, 32 insertions(+), 18 deletions(-) delete mode 100644 apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg create mode 100644 apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg diff --git a/apps/common/main/resources/img/toolbar/icons.svg b/apps/common/main/resources/img/toolbar/icons.svg index 170b65548e..fd57365d4b 100644 --- a/apps/common/main/resources/img/toolbar/icons.svg +++ b/apps/common/main/resources/img/toolbar/icons.svg @@ -287,8 +287,10 @@ - - + + diff --git a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg deleted file mode 100644 index 022488797d..0000000000 --- a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-add.svg +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg new file mode 100644 index 0000000000..ab71c0a69c --- /dev/null +++ b/apps/common/main/resources/img/toolbar/v2/2.5x/btn-nc-assistant.svg @@ -0,0 +1,5 @@ + + + \ No newline at end of file diff --git a/apps/documenteditor/main/app/view/DocumentHolderExt.js b/apps/documenteditor/main/app/view/DocumentHolderExt.js index 3b8934ffd0..f70c1fc85f 100644 --- a/apps/documenteditor/main/app/view/DocumentHolderExt.js +++ b/apps/documenteditor/main/app/view/DocumentHolderExt.js @@ -2297,7 +2297,7 @@ define([], function () { caption : '--' }); me.menuParaAssistant = new Common.UI.MenuItem({ - iconCls : 'menu__icon btn-nc-add', + iconCls : 'menu__icon btn-nc-assistant', caption : me.txtNcAssistant || 'Ask Nextcloud Assistant' }); me.menuParaAssistant.assistantSeparator = menuParaAssistantSeparator; diff --git a/apps/documenteditor/main/resources/img/toolbar/icons.svg b/apps/documenteditor/main/resources/img/toolbar/icons.svg index dc814e9b3e..b67344ca82 100644 --- a/apps/documenteditor/main/resources/img/toolbar/icons.svg +++ b/apps/documenteditor/main/resources/img/toolbar/icons.svg @@ -287,8 +287,10 @@ - - + + diff --git a/apps/pdfeditor/main/resources/img/toolbar/icons.svg b/apps/pdfeditor/main/resources/img/toolbar/icons.svg index 392dd22df8..6625b3c34d 100644 --- a/apps/pdfeditor/main/resources/img/toolbar/icons.svg +++ b/apps/pdfeditor/main/resources/img/toolbar/icons.svg @@ -287,8 +287,10 @@ - - + + diff --git a/apps/presentationeditor/main/app/view/DocumentHolderExt.js b/apps/presentationeditor/main/app/view/DocumentHolderExt.js index 327a264fad..66afccb021 100644 --- a/apps/presentationeditor/main/app/view/DocumentHolderExt.js +++ b/apps/presentationeditor/main/app/view/DocumentHolderExt.js @@ -1846,7 +1846,7 @@ define([], function () { caption : '--' }); me.menuParaAssistant = new Common.UI.MenuItem({ - iconCls : 'menu__icon btn-nc-add', + iconCls : 'menu__icon btn-nc-assistant', caption : me.txtNcAssistant || 'Ask Nextcloud Assistant' }); diff --git a/apps/presentationeditor/main/resources/img/toolbar/icons.svg b/apps/presentationeditor/main/resources/img/toolbar/icons.svg index fb81e6149b..234756a154 100644 --- a/apps/presentationeditor/main/resources/img/toolbar/icons.svg +++ b/apps/presentationeditor/main/resources/img/toolbar/icons.svg @@ -287,8 +287,10 @@ - - + + diff --git a/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js b/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js index 639bc7b5e2..bf81fbf01b 100644 --- a/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js +++ b/apps/spreadsheeteditor/main/app/view/DocumentHolderExt.js @@ -796,7 +796,7 @@ define([], function () { caption : '--' }); me.pmiAssistant = new Common.UI.MenuItem({ - iconCls : 'menu__icon btn-nc-add', + iconCls : 'menu__icon btn-nc-assistant', caption : me.txtNcAssistant || 'Ask Nextcloud Assistant' }); diff --git a/apps/spreadsheeteditor/main/resources/img/toolbar/icons.svg b/apps/spreadsheeteditor/main/resources/img/toolbar/icons.svg index 321dfe82d7..babf02ac4d 100644 --- a/apps/spreadsheeteditor/main/resources/img/toolbar/icons.svg +++ b/apps/spreadsheeteditor/main/resources/img/toolbar/icons.svg @@ -287,8 +287,10 @@ - - + + diff --git a/apps/visioeditor/main/resources/img/toolbar/icons.svg b/apps/visioeditor/main/resources/img/toolbar/icons.svg index c65a95d9a4..a728bbfebc 100644 --- a/apps/visioeditor/main/resources/img/toolbar/icons.svg +++ b/apps/visioeditor/main/resources/img/toolbar/icons.svg @@ -287,8 +287,10 @@ - - + + From b5ed4bd43f6ddf0393faee84330337dd04301f30 Mon Sep 17 00:00:00 2001 From: Christoph Schaefer Date: Tue, 18 Aug 2026 12:05:12 +0200 Subject: [PATCH 17/21] chore(build): drop the mobile index.html churn from this branch The four mobile/index.html files changed in one hashed css filename each -- a local build's output committed alongside the source changes, with nothing of this feature in it. Back to main's content. They are generated and tracked, which is what makes that happen at all; #195 stops tracking them. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer --- apps/documenteditor/mobile/index.html | 2 +- apps/presentationeditor/mobile/index.html | 2 +- apps/spreadsheeteditor/mobile/index.html | 2 +- apps/visioeditor/mobile/index.html | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/documenteditor/mobile/index.html b/apps/documenteditor/mobile/index.html index ac6874c2cc..6e4b83eaf2 100644 --- a/apps/documenteditor/mobile/index.html +++ b/apps/documenteditor/mobile/index.html @@ -218,7 +218,7 @@ width: 24px; height: 24px; fill: var(--skl-toolbar-icons); -}