Skip to content

Commit f699b5a

Browse files
rubenvdlindeConduction Release Bot
andauthored
fix(fe): gate-13 — extract four inline modals, and fix the slot typo that made four settings info panels render empty (#479)
* fix(fe): extract four inline modals, and fix the mis-named slot that made four settings info panels render empty gate-13 (modal-isolation) reported three files with inline NcModal/NcDialog markup. Fixing them surfaced a second, unrelated defect that no gate and no test could see, because its failure mode is silence. THE SLOT BUG AlwaysVisibleSection declared `<slot name="info" />`. Four callers pass `<template #info-content>`: UserGroupsConfiguration, EmailConfiguration, ArchiMateImportExport, OrganizationSynchronization All four also set `:has-info-content="true"`, so the (i) button rendered and opened a modal with nothing in it. Vue drops slot content addressed to a slot the child does not declare — no warning, no error, no failing test. The name came from CollapsibleSection, which does use `info-content`; only VersionInformation used the working `info` name. Both sections now render `<slot name="info-content"><slot name="info" /></slot>` so `info-content` wins and `info` remains a fallback. All five callers render. THE EXTRACTION src/dialogs/ChangePasswordDialog.vue <- ContactpersonenList src/dialogs/ManageUserGroupsDialog.vue <- ContactpersonenList src/modals/AlwaysVisibleSectionInfoModal.vue src/modals/CollapsibleSectionInfoModal.vue ContactpersonenList drops 1563 -> 954 lines. Password validation, the HIBP pwned-check, the debounce watcher and the group selection all move into the dialog that owns them; the parent now only opens them and reacts to events. `updateContactpersoonGroups` stays in the parent because it mutates the parent's own organisationData — the dialog reports groups up rather than reaching into it. Since both dialogs mount fresh per open, `data()` IS the state reset the parent used to spell out by hand and `beforeUnmount` IS the timeout cleanup. Every t('softwarecatalog', ...) string is preserved verbatim. Two info-modal files rather than one shared component: the two sections render materially different DOM (NcModal's own title chrome and a bare body, versus a hand-painted h2 + Close footer + ~90 lines of :deep() typography). Sharing them would need a variant flag switching between two disjoint templates and two disjoint stylesheets, and converging them would have changed one section's rendered output. BOTH DIRECTIONS A new vitest spec mounts each section with #info-content supplied. Against the pre-fix wiring: FAIL tests/vitest/sectionInfoSlot.spec.js > renders #info-content inside the info modal AssertionError: expected false to be true FAIL > prefers #info-content over #info when both are supplied Tests 2 failed | 5 passed (7) The #info case still PASSED there, which is what shows the test isolates the bug rather than the harness. After the fix: 227 passed (21 files). [gate-13] modal-isolation: FAIL - 3 file(s) -> PASS No other gate moved: 19=291, 25=41, 26=3. (An earlier baseline appeared to flip six gates; that baseline was captured while `npm ci` was still running. Re-measured with node_modules present in both arms, gate-13 is the only verdict that changes.) TOOLCHAIN Vitest could not mount an SFC: no Vue plugin, and environment 'node'. Added @vitejs/plugin-vue + jsdom as devDependencies, a @nextcloud/vue stub alongside the existing router/dialogs/l10n stubs, and made vitest.config.js an async factory so the ESM-only plugin can be dynamic-imported from a CommonJS config. The default environment stays 'node'; the new spec opts into jsdom per-file, so no existing spec changes behaviour. lint 0 errors; build compiles; 227/227 unit tests pass. * fix(settings): escape the literal placeholder braces the slot fix exposed The mis-named `info-content` slot had been hiding a second bug. Because AlwaysVisibleSection only declared `<slot name="info" />`, Vue silently dropped the four callers that passed `#info-content` — so their panels were never rendered, and nothing could fail on them. EmailConfiguration's panel documents the e-mail template placeholders: Use placeholders like {{ organization.name }} and {{ user.email }} Those braces are meant literally, but Vue compiles them as interpolation against the component, which has no `organization` and no `user`. The moment the slot name was fixed and the panel rendered for the first time it threw `Cannot read properties of undefined (reading 'name')`, which tripped the shared "no console errors" assertion in every Playwright settings test. `v-pre` keeps the braces as documentation. The other four info panels were checked and render clean. Also adds tests/vitest/settingsInfoPanels.spec.js, which renders the REAL markup of every info panel under src/views/settings/sections/. The existing sectionInfoSlot.spec.js proves the slot MECHANISM forwards content, but it does so with synthetic probe markup — which is precisely why it could not see this. Verified both ways: without `v-pre` the new spec reproduces the exact TypeError from CI. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
1 parent 274790f commit f699b5a

14 files changed

Lines changed: 2255 additions & 857 deletions

package-lock.json

Lines changed: 692 additions & 80 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,9 @@
9999
"@types/jest": "^29.5.12",
100100
"@types/node": "^20.14.12",
101101
"@typescript-eslint/parser": "^7.18.0",
102+
"@vitejs/plugin-vue": "^6.0.8",
102103
"@vitest/coverage-v8": "^3.2.7",
104+
"@vue/compiler-dom": "^3.5.41",
103105
"@vue/compiler-sfc": "^3.5.40",
104106
"@vue/eslint-config-typescript": "^13.0.0",
105107
"@vue/test-utils": "^2.4.6",
@@ -123,6 +125,7 @@
123125
"jest": "^29.0.0",
124126
"jest-environment-jsdom": "^29.7.0",
125127
"jest-transform-stub": "^2.0.0",
128+
"jsdom": "^29.1.1",
126129
"postcss-html": "^1.8.1",
127130
"stylelint": "^15.11.0",
128131
"stylelint-config-recommended-scss": "^13.1.0",

src/components/AlwaysVisibleSection.vue

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -81,26 +81,36 @@
8181
</div>
8282
</div>
8383

84-
<!-- Info Modal -->
85-
<NcModal
84+
<!-- Info Modal — own file per ADR-004/ADR-012 -->
85+
<AlwaysVisibleSectionInfoModal
8686
v-if="hasInfoContent"
87+
:name="name"
8788
:show="showInfoModal"
88-
:title="name + ' Information'"
89-
:name="name + ' Info'"
9089
@close="showInfoModal = false">
91-
<div class="info-content">
90+
<!--
91+
BOTH slot names are honoured. `info-content` wins when supplied and
92+
`info` is the fallback, so no caller can be silently empty.
93+
94+
This section only ever declared `info`, but four of its five callers
95+
(UserGroupsConfiguration, EmailConfiguration, ArchiMateImportExport,
96+
OrganizationSynchronization) pass `#info-content` — the name
97+
CollapsibleSection uses. They all set `:has-info-content="true"`, so
98+
the (i) button rendered and opened a completely EMPTY modal.
99+
-->
100+
<slot name="info-content">
92101
<slot name="info" />
93-
</div>
94-
</NcModal>
102+
</slot>
103+
</AlwaysVisibleSectionInfoModal>
95104
</NcSettingsSection>
96105
</template>
97106

98107
<script>
99108
import { defineComponent } from 'vue'
100-
import { NcSettingsSection, NcButton, NcLoadingIcon, NcModal } from '@nextcloud/vue'
109+
import { NcSettingsSection, NcButton, NcLoadingIcon } from '@nextcloud/vue'
101110
import Save from 'vue-material-design-icons/ContentSave.vue'
102111
import Refresh from 'vue-material-design-icons/Refresh.vue'
103112
import Information from 'vue-material-design-icons/Information.vue'
113+
import AlwaysVisibleSectionInfoModal from '../modals/AlwaysVisibleSectionInfoModal.vue'
104114
105115
/**
106116
* Always Visible Section component
@@ -117,10 +127,10 @@ export default defineComponent({
117127
NcSettingsSection,
118128
NcButton,
119129
NcLoadingIcon,
120-
NcModal,
121130
Save,
122131
Refresh,
123132
Information,
133+
AlwaysVisibleSectionInfoModal,
124134
},
125135
126136
props: {
@@ -311,10 +321,7 @@ export default defineComponent({
311321
color: var(--color-text-lighter);
312322
}
313323
314-
.info-content {
315-
max-width: 600px;
316-
line-height: 1.6;
317-
}
324+
/* .info-content lives with the modal in src/modals/AlwaysVisibleSectionInfoModal.vue */
318325
319326
/* Responsive */
320327
@media (max-width: 768px) {

src/components/CollapsibleSection.vue

Lines changed: 20 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -96,26 +96,23 @@
9696
</div>
9797
</div>
9898

99-
<!-- Info Modal -->
100-
<NcModal
99+
<!-- Info Modal — own file per ADR-004/ADR-012 -->
100+
<CollapsibleSectionInfoModal
101101
v-if="showInfoModal"
102+
:name="name"
102103
@close="showInfoModal = false">
103-
<div class="info-modal">
104-
<div class="modal-header">
105-
<h2>{{ name }} - Information</h2>
106-
</div>
107-
<div class="modal-content">
108-
<slot name="info-content">
109-
<p>No additional information available.</p>
110-
</slot>
111-
</div>
112-
<div class="modal-footer">
113-
<NcButton @click="showInfoModal = false">
114-
Close
115-
</NcButton>
116-
</div>
117-
</div>
118-
</NcModal>
104+
<!--
105+
BOTH slot names are honoured, matching AlwaysVisibleSection: this
106+
section has always used `info-content`, and `info` is accepted as an
107+
alias so the two sections share one slot API and no caller can be
108+
silently empty. The empty-state paragraph is the last fallback.
109+
-->
110+
<slot name="info-content">
111+
<slot name="info">
112+
<p>No additional information available.</p>
113+
</slot>
114+
</slot>
115+
</CollapsibleSectionInfoModal>
119116
</NcSettingsSection>
120117
</template>
121118

@@ -136,7 +133,6 @@ import {
136133
NcSettingsSection,
137134
NcButton,
138135
NcLoadingIcon,
139-
NcModal,
140136
} from '@nextcloud/vue'
141137
142138
// Icons
@@ -146,19 +142,21 @@ import Information from 'vue-material-design-icons/Information.vue'
146142
import ChevronUp from 'vue-material-design-icons/ChevronUp.vue'
147143
import ChevronDown from 'vue-material-design-icons/ChevronDown.vue'
148144
145+
import CollapsibleSectionInfoModal from '../modals/CollapsibleSectionInfoModal.vue'
146+
149147
export default {
150148
name: 'CollapsibleSection',
151149
152150
components: {
153151
NcSettingsSection,
154152
NcButton,
155153
NcLoadingIcon,
156-
NcModal,
157154
Save,
158155
Refresh,
159156
Information,
160157
ChevronUp,
161158
ChevronDown,
159+
CollapsibleSectionInfoModal,
162160
},
163161
164162
props: {
@@ -365,73 +363,8 @@ export default {
365363
padding: 40px 0;
366364
}
367365
368-
/* Info Modal Styles */
369-
.info-modal {
370-
padding: 20px;
371-
max-width: 600px;
372-
max-height: 80vh;
373-
overflow-y: auto;
374-
}
375-
376-
.modal-header {
377-
margin-bottom: 16px;
378-
padding-bottom: 16px;
379-
border-bottom: 1px solid var(--color-border);
380-
}
381-
382-
.modal-header h2 {
383-
margin: 0;
384-
font-size: 20px;
385-
font-weight: 600;
386-
color: var(--color-main-text);
387-
}
388-
389-
.modal-content {
390-
margin-bottom: 20px;
391-
line-height: 1.6;
392-
}
393-
394-
.modal-content :deep(h3) {
395-
margin-top: 20px;
396-
margin-bottom: 12px;
397-
font-size: 16px;
398-
font-weight: 600;
399-
}
400-
401-
.modal-content :deep(h4) {
402-
margin-top: 16px;
403-
margin-bottom: 8px;
404-
font-size: 14px;
405-
font-weight: 600;
406-
}
407-
408-
.modal-content :deep(ul) {
409-
padding-left: 20px;
410-
margin-bottom: 16px;
411-
}
412-
413-
.modal-content :deep(li) {
414-
margin-bottom: 4px;
415-
}
416-
417-
.modal-content :deep(p) {
418-
margin-bottom: 12px;
419-
}
420-
421-
.modal-content :deep(code) {
422-
background-color: var(--color-background-dark);
423-
padding: 2px 6px;
424-
border-radius: 4px;
425-
font-family: monospace;
426-
font-size: 13px;
427-
}
428-
429-
.modal-footer {
430-
display: flex;
431-
justify-content: flex-end;
432-
padding-top: 16px;
433-
border-top: 1px solid var(--color-border);
434-
}
366+
/* The info-modal styles live with the modal in
367+
src/modals/CollapsibleSectionInfoModal.vue */
435368
436369
/* WCAG 2.3.3 — the expand animation is decorative; a reduced-motion user gets
437370
the expanded section immediately instead of the slide. */

0 commit comments

Comments
 (0)