Make the settings card behave like the other plugin cards
It rendered as a flat, always-open block with no header. The framework's card
chrome is a disclosure: the tab renders every card into a <ul>, each card's root
is an <li>, its header is its own <button> that discloses the controls in place,
it starts collapsed, the header marks it when it holds unsaved edits, and a save
that lands folds it back up.
Three things were wrong and only one of them was cosmetic:
* the root was a <div> inside the tab's <ul>, so the list and card styling
never reached it
* there was no header button, so no disclosure and no aria-expanded
* nothing marked unsaved edits, and nothing collapsed after a save
`PluginCard` is not importable — it lives in that package's client bundle, and
the client purity gate forbids cross-plugin value imports — so the behaviour is
reproduced rather than borrowed, with inline styles for the same reason.
The collapse-on-save originally lived in an effect watching a `saving` flag, which
is how the shipped card does it because its form state comes from a controller.
This card awaits its own writes, so it already knows the save landed; collapsing
directly in the success path is simpler and is what the tests can actually reach.
A failed save still keeps the card open with its drafts.
Tests cover the disclosure itself: the root element, collapsed-by-default, the
header's aria-expanded and label, that staged edits survive collapsing, and that
a landed save folds the card up.
This commit is contained in:
1 parent
6b9d91b436
commit
2d7a7d1e73
3 files changed
+209
-88
No files matched your search
+98
-40
@@ -10,7 +10,8 @@
|
||||
* The card is then rendered by calling the registered component and expanding
|
||||
* the elements it returns, and its handlers are driven directly. That is not a
|
||||
* substitute for looking at it in a browser, but it does check the wiring the
|
||||
* card cannot work without.
|
||||
* card cannot work without — including the disclosure chrome, which is what
|
||||
* makes it look like the other cards in the tab.
|
||||
*/
|
||||
|
||||
import assert from 'node:assert/strict'
|
||||
@@ -19,9 +20,14 @@ import { test } from 'node:test'
|
||||
|
||||
const SOURCE = readFileSync(new URL('../client/client.js', import.meta.url), 'utf8')
|
||||
|
||||
/** Minimal React: enough for createElement plus the three hooks the card uses. */
|
||||
/**
|
||||
* Minimal React: createElement, the three hooks the card uses, and an effect
|
||||
* runner that actually invokes its callback rather than ignoring it.
|
||||
*
|
||||
* `useState` consumes one ordered slot per call, as real hooks do.
|
||||
*/
|
||||
function makeFakeReact() {
|
||||
const states = []
|
||||
const slots = []
|
||||
let cursor = 0
|
||||
return {
|
||||
reset() {
|
||||
@@ -32,15 +38,17 @@ function makeFakeReact() {
|
||||
useState(initial) {
|
||||
const index = cursor
|
||||
cursor += 1
|
||||
if (!(index in states)) states[index] = typeof initial === 'function' ? initial() : initial
|
||||
if (!(index in slots)) slots[index] = typeof initial === 'function' ? initial() : initial
|
||||
return [
|
||||
states[index],
|
||||
slots[index],
|
||||
(next) => {
|
||||
states[index] = typeof next === 'function' ? next(states[index]) : next
|
||||
slots[index] = typeof next === 'function' ? next(slots[index]) : next
|
||||
},
|
||||
]
|
||||
},
|
||||
useEffect() {},
|
||||
useEffect(effect) {
|
||||
effect()
|
||||
},
|
||||
useMemo(factory) {
|
||||
return factory()
|
||||
},
|
||||
@@ -62,12 +70,7 @@ function loadBundle() {
|
||||
return { id: loaded.id, exported, react }
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve function components by calling them, until a host element or null.
|
||||
*
|
||||
* This is the whole "renderer": the slot hands back an element whose type is the
|
||||
* card component, and function-typed children are expanded the same way.
|
||||
*/
|
||||
/** Resolve function components by calling them, until a host element or null. */
|
||||
function expand(node, depth = 0) {
|
||||
if (depth > 50) throw new Error('the component tree did not settle')
|
||||
if (node === null || node === undefined) return null
|
||||
@@ -150,10 +153,22 @@ function registered() {
|
||||
},
|
||||
}
|
||||
exported.apply(ctx)
|
||||
const draw = (scope) => expand(render({ scope }))
|
||||
return { id, exported, react, registration, render, draw }
|
||||
const draw = (scope) => {
|
||||
react.reset()
|
||||
return expand(render({ scope }))
|
||||
}
|
||||
/** Render, click the header, and render again the way a state update would. */
|
||||
const drawOpen = (scope) => {
|
||||
const header = headerOf(draw(scope))
|
||||
header.props.onClick()
|
||||
return draw(scope)
|
||||
}
|
||||
return { id, exported, react, registration, draw, drawOpen }
|
||||
}
|
||||
|
||||
const headerOf = (tree) =>
|
||||
findAll(tree, (node) => node.type === 'button' && node.props['aria-expanded'] !== undefined)[0]
|
||||
|
||||
const inputFor = (tree, label) =>
|
||||
findAll(tree, (node) => node.type === 'input' && node.props['aria-label'] === label)[0]
|
||||
|
||||
@@ -179,9 +194,25 @@ test('applying registers exactly one settings.plugin.item card, keyed on the nam
|
||||
assert.equal(registration.key, 'dsh-slopo')
|
||||
})
|
||||
|
||||
test('the card renders a control for every setting the Host accepts', () => {
|
||||
test('the root is an <li>, because the tab renders every card into a <ul>', () => {
|
||||
const { draw } = registered()
|
||||
assert.equal(draw(fakeScope(READY)).type, 'li')
|
||||
})
|
||||
|
||||
test('the card starts collapsed, with a header that says so', () => {
|
||||
const { draw } = registered()
|
||||
const tree = draw(fakeScope(READY))
|
||||
assert.equal(headerOf(tree).props['aria-expanded'], false)
|
||||
assert.match(headerOf(tree).props['aria-label'], /^Expand: /)
|
||||
// Nothing is disclosed yet.
|
||||
assert.equal(findAll(tree, (node) => node.type === 'input').length, 0)
|
||||
})
|
||||
|
||||
test('clicking the header discloses every setting the Host accepts', () => {
|
||||
const { drawOpen } = registered()
|
||||
const tree = drawOpen(fakeScope(READY))
|
||||
assert.equal(headerOf(tree).props['aria-expanded'], true)
|
||||
assert.match(headerOf(tree).props['aria-label'], /^Collapse: /)
|
||||
const labels = findAll(tree, (node) => node.type === 'label').map(textOf)
|
||||
for (const expected of [
|
||||
'Embedding model',
|
||||
@@ -204,8 +235,8 @@ test('the card renders a control for every setting the Host accepts', () => {
|
||||
})
|
||||
|
||||
test('the API key is rendered as a password field', () => {
|
||||
const { draw } = registered()
|
||||
const tree = draw(fakeScope(READY))
|
||||
const { drawOpen } = registered()
|
||||
const tree = drawOpen(fakeScope(READY))
|
||||
assert.equal(inputFor(tree, 'Embedding API key').props.type, 'password')
|
||||
})
|
||||
|
||||
@@ -214,72 +245,98 @@ test('the card renders nothing when the namespace is unavailable', () => {
|
||||
assert.equal(draw(fakeScope({ ...READY, status: 'unavailable' })), null)
|
||||
})
|
||||
|
||||
test('editing a field marks the card dirty and saving writes it through set()', async () => {
|
||||
const { draw, react } = registered()
|
||||
test('editing a field marks the header unsaved, and saving writes it through set()', async () => {
|
||||
const { draw, drawOpen, react } = registered()
|
||||
const scope = fakeScope(READY)
|
||||
|
||||
inputFor(draw(scope), 'Embedding model').props.onChange({ target: { value: 'ollama/example:latest' } })
|
||||
const open = drawOpen(scope)
|
||||
inputFor(open, 'Embedding model').props.onChange({ target: { value: 'ollama/example:latest' } })
|
||||
|
||||
// Re-render the way a state update would.
|
||||
react.reset()
|
||||
const tree = draw(scope)
|
||||
// Marked on the header, so it is visible while collapsed.
|
||||
assert.match(textOf(headerOf(tree)), /unsaved/)
|
||||
const save = buttonStartingWith(tree, 'Save')
|
||||
assert.equal(save.props.disabled, false, 'a changed field must enable Save')
|
||||
await save.props.onClick()
|
||||
|
||||
assert.deepEqual(scope.calls.set, [['embeddingModel', 'ollama/example:latest']])
|
||||
assert.deepEqual(scope.calls.unset, [])
|
||||
assert.equal(react !== undefined, true)
|
||||
})
|
||||
|
||||
test('a successful save collapses the card again', async () => {
|
||||
const { draw, drawOpen } = registered()
|
||||
const scope = fakeScope(READY)
|
||||
inputFor(drawOpen(scope), 'Embedding model').props.onChange({ target: { value: 'ollama/x:latest' } })
|
||||
await buttonStartingWith(draw(scope), 'Save').props.onClick()
|
||||
|
||||
// The save sets busy=false and note='Saved.', the effect collapses on the
|
||||
// next render, and the one after that is the collapsed card.
|
||||
draw(scope)
|
||||
const settled = draw(scope)
|
||||
assert.equal(headerOf(settled).props['aria-expanded'], false, 'a landed save closes the card')
|
||||
assert.equal(findAll(settled, (node) => node.type === 'input').length, 0)
|
||||
})
|
||||
|
||||
test('staged edits survive collapsing', () => {
|
||||
const { draw, drawOpen } = registered()
|
||||
const scope = fakeScope(READY)
|
||||
inputFor(drawOpen(scope), 'Embedding model').props.onChange({ target: { value: 'ollama/kept:latest' } })
|
||||
// Collapse it by hand.
|
||||
headerOf(draw(scope)).props.onClick()
|
||||
const collapsed = draw(scope)
|
||||
assert.equal(headerOf(collapsed).props['aria-expanded'], false)
|
||||
assert.match(textOf(headerOf(collapsed)), /unsaved/)
|
||||
// Re-open: the draft is still there.
|
||||
headerOf(collapsed).props.onClick()
|
||||
assert.equal(inputFor(draw(scope), 'Embedding model').props.value, 'ollama/kept:latest')
|
||||
})
|
||||
|
||||
test('a whole number field is sent as a number, not the string the input holds', async () => {
|
||||
const { draw, react } = registered()
|
||||
const { draw, drawOpen } = registered()
|
||||
const scope = fakeScope(READY)
|
||||
inputFor(draw(scope), 'Embedding dimensions').props.onChange({ target: { value: '768' } })
|
||||
react.reset()
|
||||
inputFor(drawOpen(scope), 'Embedding dimensions').props.onChange({ target: { value: '768' } })
|
||||
await buttonStartingWith(draw(scope), 'Save').props.onClick()
|
||||
assert.deepEqual(scope.calls.set, [['embeddingDimensions', 768]])
|
||||
})
|
||||
|
||||
test('clearing a field is an unset, so it re-inherits rather than becoming empty', async () => {
|
||||
const { draw, react } = registered()
|
||||
const { draw, drawOpen } = registered()
|
||||
const scope = fakeScope({ ...READY, value: { embeddingModel: 'ollama/old:latest', embeddingDimensions: 768 } })
|
||||
inputFor(draw(scope), 'Embedding model').props.onChange({ target: { value: '' } })
|
||||
react.reset()
|
||||
inputFor(drawOpen(scope), 'Embedding model').props.onChange({ target: { value: '' } })
|
||||
await buttonStartingWith(draw(scope), 'Save').props.onClick()
|
||||
assert.deepEqual(scope.calls.unset, ['embeddingModel'])
|
||||
assert.deepEqual(scope.calls.set, [], 'a cleared field must not be written as an empty string')
|
||||
})
|
||||
|
||||
test('an exclude list round-trips through comma-separated text', async () => {
|
||||
const { draw, react } = registered()
|
||||
const { draw, drawOpen } = registered()
|
||||
const scope = fakeScope({ ...READY, value: { embeddingModel: 'm', embeddingDimensions: 1 } })
|
||||
inputFor(draw(scope), 'Extra exclude patterns').props.onChange({ target: { value: '**/test/**, *.min.js' } })
|
||||
react.reset()
|
||||
inputFor(drawOpen(scope), 'Extra exclude patterns').props.onChange({ target: { value: '**/test/**, *.min.js' } })
|
||||
await buttonStartingWith(draw(scope), 'Save').props.onClick()
|
||||
assert.deepEqual(scope.calls.set, [['sourceDirExclude', ['**/test/**', '*.min.js']]])
|
||||
})
|
||||
|
||||
test('a boolean field writes a real boolean', async () => {
|
||||
const { draw, react } = registered()
|
||||
const { draw, drawOpen } = registered()
|
||||
const scope = fakeScope(READY)
|
||||
const checkbox = findAll(
|
||||
draw(scope),
|
||||
drawOpen(scope),
|
||||
(node) => node.type === 'input' && node.props['aria-label'] === 'Install slopo with uv when missing',
|
||||
)[0]
|
||||
checkbox.props.onChange({ target: { checked: true } })
|
||||
react.reset()
|
||||
await buttonStartingWith(draw(scope), 'Save').props.onClick()
|
||||
assert.deepEqual(scope.calls.set, [['autoInstall', true]])
|
||||
})
|
||||
|
||||
test('an overridden field is marked, and Reset clears every override', async () => {
|
||||
const { draw } = registered()
|
||||
const { drawOpen } = registered()
|
||||
const scope = fakeScope({
|
||||
...READY,
|
||||
value: { embeddingModel: 'ollama/x:latest', embeddingDimensions: 768 },
|
||||
user: { embeddingModel: 'ollama/x:latest', embeddingDimensions: 768 },
|
||||
})
|
||||
const tree = draw(scope)
|
||||
const tree = drawOpen(scope)
|
||||
const labels = findAll(tree, (node) => node.type === 'label').map(textOf)
|
||||
assert.ok(labels.some((label) => label.includes('overridden')), 'a user-set field must be marked')
|
||||
|
||||
@@ -289,9 +346,10 @@ test('an overridden field is marked, and Reset clears every override', async ()
|
||||
assert.deepEqual(scope.calls.unset.sort(), ['embeddingDimensions', 'embeddingModel'])
|
||||
})
|
||||
|
||||
test('a read-only namespace disables every control', () => {
|
||||
const { draw } = registered()
|
||||
const tree = draw(fakeScope({ ...READY, writable: false, mode: 'memory' }))
|
||||
test('a read-only namespace says so and disables every control', () => {
|
||||
const { drawOpen } = registered()
|
||||
const tree = drawOpen(fakeScope({ ...READY, writable: false, mode: 'memory' }))
|
||||
assert.match(textOf(tree), /read-only/)
|
||||
const inputs = findAll(tree, (node) => node.type === 'input')
|
||||
assert.ok(inputs.length > 0)
|
||||
for (const input of inputs) assert.equal(input.props.disabled, true)
|
||||
|
||||
Reference in new issue
Block a user