diff --git a/package-lock.json b/package-lock.json index 060f89e..aad813e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,16 +1,16 @@ { "name": "simulink-data-explorer", - "version": "1.16.0", + "version": "1.17.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "simulink-data-explorer", - "version": "1.16.0", + "version": "1.17.0", "license": "BSD-3-Clause", "dependencies": { "@lit/context": "^1.1.6", - "data-explorer-core": "github:mathworks/data-explorer-core#v1.14.0", + "data-explorer-core": "github:mathworks/data-explorer-core#v1.15.0", "fast-xml-parser": "^5.8.0", "fflate": "^0.8.3", "highlight.js": "^11.11.1", @@ -3184,8 +3184,8 @@ } }, "node_modules/data-explorer-core": { - "version": "1.14.0", - "resolved": "git+ssh://git@github.com/mathworks/data-explorer-core.git#3c3f0645e36cfc74ab05901a83caeeb812434c85", + "version": "1.15.0", + "resolved": "git+ssh://git@github.com/mathworks/data-explorer-core.git#45375876def00580e8d00a3afaa3a476ec689d4c", "license": "BSD-3-Clause", "dependencies": { "fast-xml-parser": "^5.8.0", diff --git a/package.json b/package.json index 3fe6c12..7a4bff5 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "simulink-data-explorer", "displayName": "Simulink Data Explorer", "description": "Explore Simulink models, data dictionaries, MAT-files, and projects as interactive tables and relationship trees.", - "version": "1.16.0", + "version": "1.17.0", "publisher": "mathworks", "icon": "media/icon.png", "private": true, @@ -195,7 +195,7 @@ }, "dependencies": { "@lit/context": "^1.1.6", - "data-explorer-core": "github:mathworks/data-explorer-core#v1.14.0", + "data-explorer-core": "github:mathworks/data-explorer-core#v1.15.0", "fast-xml-parser": "^5.8.0", "fflate": "^0.8.3", "highlight.js": "^11.11.1", diff --git a/src/webview/components/dex-tree-table.ts b/src/webview/components/dex-tree-table.ts index 741b728..33dc334 100644 --- a/src/webview/components/dex-tree-table.ts +++ b/src/webview/components/dex-tree-table.ts @@ -221,6 +221,17 @@ export interface BlockLinkGroup { blocks: { blockName: string; blockPath: string; linkTarget: string }[]; } +// One entry of a `paramLinks` cell, as the host's usageCells.ParamLink builds it. +// `linkTarget` is EMPTY for a parameter whose value the graph could not resolve to +// anything in this workspace, which is a normal answer and not a defect — see +// `_renderParamLinks`, the one place that decides what an empty target renders as. +interface ParamLinkCell { + property: string; + paramName: string; + source: string; + linkTarget: string; +} + // A Usage cell's block links, one group per model: `AFR, AFRMonitor, // MixTarget(EngineCtrl); AFRConst, AFRCheck(FuelInjector)`. A dictionary variable is // used by as many blocks as a model has, so naming the model once per block spent most @@ -2811,6 +2822,32 @@ export class DexTreeTable extends LitElement { return html``; } + // A `paramLinks` list — `Gain=Kp (dict.sldd)` — rendered once for the two columns + // that carry one (DataType and UsedBy). It used to be spelled twice, which is how + // it came to disagree with itself: the Value branch above gates its anchor on + // `linkTarget`, both copies of this did not, and so a param the graph could NOT + // resolve still painted accent-blue, underlined on hover, and dispatched a click + // carrying an empty target that routes nowhere. Three ways of saying "link" and + // nothing behind any of them. + // + // An unresolved param is not a rendering accident to hide — MATLAB itself reports + // `Gain = finalGain` as a real variable reference when `finalGain` lives in the BASE + // workspace, which is a live MATLAB session and not a file anything here can open. + // So the value stays visible and only its linkhood goes. The TEXT is untouched + // either way (`_getCellText` builds it from the same list), so what a user sorts, + // filters and copies does not depend on whether the target resolved. + private _renderParamLinks(paramLinks: ParamLinkCell[], columnId: string): unknown { + return html`${paramLinks.map((p, i) => { + const name = this._highlight(p.paramName, columnId); + return html`${i > 0 ? ', ' : ''}${p.property + '='}${p.linkTarget + ? html` this._onLinkClick(p.linkTarget, e)} + >${name}` + : name}${p.source ? html`${'(' + p.source + ')'}` : ''}`; + })}`; + } + private _renderCellValue(row: TreeTableRow, columnId: string): unknown { const isEditing = this._editingCell?.rowId === row.ID && this._editingCell?.columnId === columnId; @@ -2908,13 +2945,7 @@ export class DexTreeTable extends LitElement { if (columnId === 'DataType') { const val = isCellObject(row.DataType) ? (row.DataType as any) : { text: cellText(row.DataType) }; if ('paramLinks' in val) { - return html`${val.paramLinks.map( - (p: { property: string; paramName: string; source: string; linkTarget: string }, i: number) => - html`${i > 0 ? ', ' : ''}${p.property + '='} this._onLinkClick(p.linkTarget, e)} - >${this._highlight(p.paramName, columnId)}${p.source ? html`${'(' + p.source + ')'}` : ''}`, - )}`; + return this._renderParamLinks(val.paramLinks, columnId); } if ('links' in val) { return html`${val.links.map( @@ -2964,13 +2995,7 @@ export class DexTreeTable extends LitElement { if (!row.UsedBy) return html``; const val = isCellObject(row.UsedBy) ? (row.UsedBy as any) : { text: cellText(row.UsedBy) }; if ('paramLinks' in val) { - return html`${(val as any).paramLinks.map( - (p: { property: string; paramName: string; source: string; linkTarget: string }, i: number) => - html`${i > 0 ? ', ' : ''}${p.property + '='} this._onLinkClick(p.linkTarget, e)} - >${this._highlight(p.paramName, columnId)}${p.source ? html`${'(' + p.source + ')'}` : ''}`, - )}`; + return this._renderParamLinks((val as any).paramLinks, columnId); } if ('blockLinks' in val) { // One `(model)` per group, after its last block. The qualifier is dropped diff --git a/test/treeTableCells.test.ts b/test/treeTableCells.test.ts index 413293c..0fd2883 100644 --- a/test/treeTableCells.test.ts +++ b/test/treeTableCells.test.ts @@ -332,6 +332,100 @@ describe('links navigate rather than following an href', () => { table.remove(); }); + // A param the graph could not resolve is a NORMAL answer, not a defect to hide. MATLAB + // itself calls `Gain = finalGain` a real variable reference when `finalGain` lives in + // the BASE workspace — a live MATLAB session, which is not a file anything here can + // open. Every other unresolved case (a Bus Selector's `OutputSignals = a,b`, a Math + // block's `Operator = square`) has nothing to open either. + // + // So the value must READ but not CLAIM to be clickable. It used to claim it: both + // paramLinks columns wrote the anchor unconditionally while the Value column next to + // them gated on `linkTarget`, so an unresolvable param painted accent-blue, underlined + // on hover, and fired a navigation carrying an empty target. One rule, three spellings. + describe('a param the graph could not resolve is text, not a dead link', () => { + const UNRESOLVED = { property: 'Gain', paramName: 'finalGain', source: '', linkTarget: '' }; + + it('renders no anchor, in either column that carries paramLinks', async () => { + const table = await mount([ + makeRow('u', 'u', { UsedBy: { paramLinks: [UNRESOLVED] } as any }), + makeRow('d', 'd', { DataType: { paramLinks: [UNRESOLVED] } as any }), + ]); + expect(cell(table, 'u', 'UsedBy').querySelector('a.value-link')).toBeNull(); + expect(cell(table, 'd', 'DataType').querySelector('a.value-link')).toBeNull(); + table.remove(); + }); + + it('still shows the property and the value, so nothing is lost by not linking it', async () => { + const table = await mount([makeRow('u', 'u', { UsedBy: { paramLinks: [UNRESOLVED] } as any })]); + const td = cell(table, 'u', 'UsedBy'); + expect(td.querySelector('.param-property')!.textContent).toBe('Gain='); + expect(text(table, 'u', 'UsedBy')).toBe('Gain=finalGain'); + table.remove(); + }); + + it('leaves the cell TEXT identical to a resolved one, so sorting and copying do not shift', async () => { + // The text comes from `_getCellText`, which reads the same list and knows nothing + // about targets. Gating the anchor must not become a second, quieter answer to + // "what does this cell say". + const table = await mount([ + makeRow('n', 'n', { UsedBy: { paramLinks: [UNRESOLVED] } as any }), + makeRow('y', 'y', { UsedBy: { paramLinks: [{ ...UNRESOLVED, linkTarget: 'workspace:finalGain@f' }] } as any }), + ]); + const [n, y] = table.rows; + expect((table as any)._getCellText(n, 'UsedBy')).toBe((table as any)._getCellText(y, 'UsedBy')); + table.remove(); + }); + + it('links only the resolved entries of a cell that holds both', async () => { + // The ordinary case for a block: `Operator=square` resolves to nothing while the + // gain beside it resolves to a dictionary. A cell that linked both would send one + // of the two clicks nowhere; a cell that linked neither would lose a real jump. + const table = await mount([ + makeRow('u', 'u', { + UsedBy: { + paramLinks: [ + { property: 'Operator', paramName: 'square', source: '', linkTarget: '' }, + { property: 'Gain', paramName: 'Kp', source: 'params.sldd', linkTarget: 'Kp@file:///w/params.sldd' }, + ], + } as any, + }), + ]); + const td = cell(table, 'u', 'UsedBy'); + const links = Array.from(td.querySelectorAll('a.value-link')); + expect(links.map((a) => a.textContent!.trim())).toEqual(['Kp']); + expect(text(table, 'u', 'UsedBy')).toBe('Operator=square, Gain=Kp(params.sldd)'); + table.remove(); + }); + + it('cannot dispatch a navigation with an empty target', async () => { + // The defect as the user met it: the value looked like a link, the click was + // swallowed by preventDefault, and `dex-link-clicked` carried '' — which routes + // nowhere, so nothing happened at all. + const table = await mount([makeRow('u', 'u', { UsedBy: { paramLinks: [UNRESOLVED] } as any })]); + const clicked: string[] = []; + table.addEventListener('dex-link-clicked', (e) => clicked.push((e as CustomEvent).detail.target)); + // Whatever the cell offers as clickable, clicking it must not ask the host to + // navigate to nothing. Asserted by exercising every link in the cell rather than + // by counting them, so this stays a statement about behaviour and not about markup. + for (const a of Array.from(cell(table, 'u', 'UsedBy').querySelectorAll('a.value-link'))) { + (a as HTMLElement).click(); + } + expect(clicked).toEqual([]); + table.remove(); + }); + + it('is still highlighted by a search that matches it', async () => { + // Highlighting is about finding the text, which is there whether or not it links. + const table = await mount([makeRow('u', 'u', { UsedBy: { paramLinks: [UNRESOLVED] } as any })]); + const input = table.shadowRoot!.querySelector('.filter-input') as HTMLInputElement; + input.value = 'final'; + input.dispatchEvent(new Event('input', { bubbles: true })); + await table.updateComplete; + expect(cell(table, 'u', 'UsedBy').querySelector('mark')!.textContent).toBe('final'); + table.remove(); + }); + }); + it('blockLinks name the block and its model', async () => { // Two models can hold blocks with the same name, so the model qualifier is // what makes the reference identifiable.