fix(ZMSKVR-1228): keyboard usable modal#1933
fix(ZMSKVR-1228): keyboard usable modal#1933ChristianVierthaler wants to merge 10 commits intonextfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors AvailabilityInfoModal to use Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
zmscitizenview/src/components/Appointment/AppointmentSelection.vue (1)
677-680:closeAvailabilityInfoModalis now dead code — remove it.With
@close="closeAvailabilityInfoModal"removed from the template, this function has no callers. The v-model binding handlesshowAvailabilityInfoModal.value = false, andavailabilityInfoHtmlOverrideis already cleared on the next call toopenAvailabilityInfoModal. Safe to delete.🧹 Proposed fix
-const closeAvailabilityInfoModal = () => { - showAvailabilityInfoModal.value = false; - availabilityInfoHtmlOverride.value = ""; -}; - const availabilityInfoHtml = computed(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmscitizenview/src/components/Appointment/AppointmentSelection.vue` around lines 677 - 680, The function closeAvailabilityInfoModal is unused/dead code — remove the entire function definition (const closeAvailabilityInfoModal = () => { ... }) from AppointmentSelection.vue; ensure there are no remaining references to closeAvailabilityInfoModal elsewhere (the template now uses v-model on showAvailabilityInfoModal and openAvailabilityInfoModal already clears availabilityInfoHtmlOverride), so deleting this declaration is safe.zmscitizenview/tests/setup/localStorageMock.ts (1)
43-55: PatchingHTMLElement.prototypeis overly broad — targetHTMLDialogElement.prototypeinstead.
showModalandcloseare nativeHTMLDialogElementmethods. Attaching them toHTMLElement.prototypeinjects the mocks on every element in the DOM during tests. Since setup files run inside the configured jsdom environment,HTMLDialogElementis available and should be the target:-if (typeof window !== 'undefined') { - Object.defineProperty(HTMLElement.prototype, 'showModal', { +if (typeof window !== 'undefined' && typeof HTMLDialogElement !== 'undefined') { + Object.defineProperty(HTMLDialogElement.prototype, 'showModal', { configurable: true, writable: true, value: vi.fn(), }); - Object.defineProperty(HTMLElement.prototype, 'close', { + Object.defineProperty(HTMLDialogElement.prototype, 'close', { configurable: true, writable: true, value: vi.fn(), }); }Also note that
vi.fn()instances are created once at module initialisation and shared across all tests in a worker. If individual tests assert call counts on these mocks, state can leak between tests. Consider either callingvi.fn()inside abeforeEachreset or usingvi.spyOnper test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmscitizenview/tests/setup/localStorageMock.ts` around lines 43 - 55, The test setup incorrectly patches HTMLElement.prototype for the dialog APIs; change the mock target to HTMLDialogElement.prototype and replace the module-level vi.fn() assignments for showModal and close with per-test setup—either create the vi.fn() mocks inside a beforeEach (or reset them in beforeEach) or prefer using vi.spyOn on HTMLDialogElement.prototype per test—so the symbols to update are HTMLElement.prototype -> HTMLDialogElement.prototype, the mocked methods showModal and close, and the vi.fn() usage (move into beforeEach or switch to vi.spyOn) to avoid cross-test state leakage.zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue (1)
38-44:dialogAriaLabelprop is declared but never used — remove it.The prop
dialogAriaLabel(line 42) has no reference in the template; accessibility labelling is handled viaaria-labelledby="modalTitle"on<muc-modal>. Keeping unused props widens the public API surface unnecessarily.🧹 Proposed fix
const props = defineProps<{ open: boolean; html: string; closeAriaLabel?: string; - dialogAriaLabel?: string; t?: (key: string) => string; }>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue` around lines 38 - 44, Remove the unused dialogAriaLabel prop from the component's props definition to shrink the public API: edit the defineProps block in AvailabilityInfoModal.vue and delete the dialogAriaLabel?: string; entry (leaving open, html, closeAriaLabel, t unchanged), then search the component for any references to dialogAriaLabel and remove them (there should be none; aria labeling is handled via aria-labelledby="modalTitle" on <muc-modal>), and update any related TypeScript types or external references if they exist.zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts (1)
26-27: Remove noisy inline comment from stub templateThe comment
<!-- props direkt verwenden, nicht props.closeAriaLabel -->is implementation noise embedded inside a template string — it explains a workaround without adding value to someone reading the test later. As per coding guidelines, comments should be avoided when the code is self-explanatory, and closing/noise comments should be removed.♻️ Proposed fix
- <!-- props direkt verwenden, nicht props.closeAriaLabel --> <button class="modal-button-close" :aria-label="closeAriaLabel || 'Dialog schließen'" type="button"></button>As per coding guidelines, "Don't add obvious noise" and "Always try to explain yourself in code."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts` around lines 26 - 27, Remove the noisy inline HTML comment <!-- props direkt verwenden, nicht props.closeAriaLabel --> from the stub/template used in the AvailabilityInfoModal unit test; locate the template string or stub in AvailabilityInfoModal.spec.ts (the test's stub/template for the modal) and delete that comment so the template is clean and self-explanatory, leaving the surrounding markup and any props references (e.g., props.closeAriaLabel) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue`:
- Around line 2-7: The template binds v-model:open to props.open which attempts
to assign into a readonly prop; change to a writable proxy: create a writable
computed (e.g., const computedOpen = computed({ get: () => props.open, set: v =>
emit('update:open', v) }) in the <script setup>) and use
v-model:open="computedOpen" on <muc-modal>, or alternatively bind
:open="props.open" and `@update`:open="val => emit('update:open', val)"; update
references to props.open accordingly so MucModal emits correctly and no readonly
assignment occurs.
In `@zmscitizenview/tests/unit/Appointment/AppointmentSelection.spec.ts`:
- Line 1816: The test is setting a non-existent prop with wrapper.setProps({
open: true }) which leaves the modal closed; instead set the component's local
ref directly by assigning wrapper.vm.showAvailabilityInfoModal = true and then
await nextTick() to ensure reactivity updates, and where you do use
wrapper.setProps(...) elsewhere (e.g., the other occurrence noted) make it
awaited (await wrapper.setProps({...})) rather than fire-and-forget to ensure
the DOM updates before assertions.
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Around line 85-87: The test named 'has modal-dialog with centered class' is
stale because it only checks for '.modal-content'; either rename the test to
reflect that assertion or update the assertions to actually verify the modal
structure: locate the test with the string 'has modal-dialog with centered
class' in AvailabilityInfoModal.spec.ts and replace its body to expect a
'.modal-dialog' element and that it contains the expected centered class (e.g.,
'.modal-dialog.centered' or the actual class used by AvailabilityInfoModal), or
change the test name to e.g. 'renders modal content' to match the existing
expect(wrapper.find(".modal-content").exists()).
- Around line 17-53: The MucModal stub in AvailabilityInfoModal.spec.ts declares
the open prop but ignores it, so the modal always renders; update the MucModal
stub template to conditionally render its content based on the open prop (e.g.,
wrap the modal-content template in a v-if="open" or equivalent) so prop-driven
visibility behaves like the real component, and then change the "does not render
when open is false" test to assert the modal is not visible/not present (use
wrapper.find('.modal-content').exists() or isVisible()) instead of merely
checking wrapper.props('open') === false; ensure the "renders when open is true"
test still checks visibility.
- Around line 122-129: The test is self-fulfilling because it directly emits the
event instead of wiring the stub's close button to trigger the component's
update:open handler; update the test for AvailabilityInfoModal so the stubbed
close button actually emits 'update:open' with false when clicked (i.e.,
configure the stub to call wrapper.vm.$emit('update:open', false) on its click
or use Vue Test Utils' listeners to forward the event), remove the manual await
wrapper.vm.$emit('update:open', false) call, trigger
closeButton.trigger('click'), await nextTick/DOM updates, and then assert that
wrapper emitted('update:open') with payload [false]; also rename the test
description from "emits close event" to "emits update:open with false when close
clicked".
In `@zmscitizenview/vitest.config.ts`:
- Around line 5-13: Remove the unreachable browser-only block starting with "if
(typeof window !== \"undefined\") { ... }" in vitest.config.ts (which defines
HTMLDialogElement and calls showModal/close via vi.fn()) because Vitest loads
this file in Node where window is undefined and the mocks are already
implemented in localStorageMock.ts; delete that entire block and any dangling
references to vi in this file (or add a proper import if vi is needed elsewhere)
so the config no longer contains dead code or an undeclared vi reference.
---
Nitpick comments:
In `@zmscitizenview/src/components/Appointment/AppointmentSelection.vue`:
- Around line 677-680: The function closeAvailabilityInfoModal is unused/dead
code — remove the entire function definition (const closeAvailabilityInfoModal =
() => { ... }) from AppointmentSelection.vue; ensure there are no remaining
references to closeAvailabilityInfoModal elsewhere (the template now uses
v-model on showAvailabilityInfoModal and openAvailabilityInfoModal already
clears availabilityInfoHtmlOverride), so deleting this declaration is safe.
In
`@zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue`:
- Around line 38-44: Remove the unused dialogAriaLabel prop from the component's
props definition to shrink the public API: edit the defineProps block in
AvailabilityInfoModal.vue and delete the dialogAriaLabel?: string; entry
(leaving open, html, closeAriaLabel, t unchanged), then search the component for
any references to dialogAriaLabel and remove them (there should be none; aria
labeling is handled via aria-labelledby="modalTitle" on <muc-modal>), and update
any related TypeScript types or external references if they exist.
In `@zmscitizenview/tests/setup/localStorageMock.ts`:
- Around line 43-55: The test setup incorrectly patches HTMLElement.prototype
for the dialog APIs; change the mock target to HTMLDialogElement.prototype and
replace the module-level vi.fn() assignments for showModal and close with
per-test setup—either create the vi.fn() mocks inside a beforeEach (or reset
them in beforeEach) or prefer using vi.spyOn on HTMLDialogElement.prototype per
test—so the symbols to update are HTMLElement.prototype ->
HTMLDialogElement.prototype, the mocked methods showModal and close, and the
vi.fn() usage (move into beforeEach or switch to vi.spyOn) to avoid cross-test
state leakage.
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Around line 26-27: Remove the noisy inline HTML comment <!-- props direkt
verwenden, nicht props.closeAriaLabel --> from the stub/template used in the
AvailabilityInfoModal unit test; locate the template string or stub in
AvailabilityInfoModal.spec.ts (the test's stub/template for the modal) and
delete that comment so the template is clean and self-explanatory, leaving the
surrounding markup and any props references (e.g., props.closeAriaLabel)
unchanged.
| <muc-modal | ||
| v-model:open="props.open" | ||
| @close="emit('update:open', false)" | ||
| aria-labelledby="modalTitle" | ||
| :closeAriaLabel="closeAriaLabel || 'Dialog schließen'" | ||
| > |
There was a problem hiding this comment.
v-model:open="props.open" mutates a readonly prop — Vue 3 will warn and the update may silently fail.
v-model:open compiles to :open="props.open" plus @update:open="props.open = $event". In Vue 3's Composition API, props is a shallow readonly proxy, so the assignment triggers a runtime warning (Set operation on key "open": target is readonly) and the value is not written. If MucModal emits update:open internally (e.g., when the user presses Escape), those updates are dropped, potentially breaking the keyboard-close flow that this PR is specifically fixing.
The idiomatic fix is a writable computed ref:
🔧 Proposed fix — writable computed for v-model passthrough
In <script setup>:
+import { computed } from "vue";
import { computed } from "vue";
...
+const isOpen = computed({
+ get: () => props.open,
+ set: (val: boolean) => emit('update:open', val),
+});In <template>:
- v-model:open="props.open"
- `@close`="emit('update:open', false)"
+ v-model:open="isOpen"
+ `@close`="isOpen = false"Or, without a computed, split binding and handler explicitly:
- v-model:open="props.open"
- `@close`="emit('update:open', false)"
+ :open="props.open"
+ `@update`:open="emit('update:open', $event)"
+ `@close`="emit('update:open', false)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue`
around lines 2 - 7, The template binds v-model:open to props.open which attempts
to assign into a readonly prop; change to a writable proxy: create a writable
computed (e.g., const computedOpen = computed({ get: () => props.open, set: v =>
emit('update:open', v) }) in the <script setup>) and use
v-model:open="computedOpen" on <muc-modal>, or alternatively bind
:open="props.open" and `@update`:open="val => emit('update:open', val)"; update
references to props.open accordingly so MucModal emits correctly and no readonly
assignment occurs.
zmscitizenview/tests/unit/Appointment/AppointmentSelection.spec.ts
Outdated
Show resolved
Hide resolved
zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts
Show resolved
Hide resolved
zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts
Outdated
Show resolved
Hide resolved
zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts (1)
23-24: Redundantv-showon.modal-content— the outerv-ifalready gates the whole subtreeWhen
openisfalse, thev-if="open"on the outer.modaldiv prevents the entire subtree from being mounted;.modal-contentnever enters the DOM and thev-show="open"on it is therefore dead code. Whenopenistrue,v-show="open"is alwaystrueand has no effect.♻️ Proposed simplification
- <div class="modal-content" v-show="open"> + <div class="modal-content">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts` around lines 23 - 24, Remove the redundant v-show on the `.modal-content` element since the outer `<div class="modal" role="dialog" aria-modal="true" v-if="open">` already controls mounting; locate the `<div class="modal-content" v-show="open">` (in AvailabilityInfoModal template) and delete the v-show="open" attribute so the markup becomes `<div class="modal-content">`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Line 28: Fix the broken `@click` expression and remove the $parent anti-pattern:
update the button in the stub (the element using `@click` that currently reads
"$emit('update:open', false; $parent.$emit('update:open', false)") to simply
emit the event itself (call $emit('update:open', false)) so there's no
semicolon-in-argument syntax error and no $parent access; also remove the
redundant v-show="open" from the .modal-content div since the parent .modal uses
v-if="open". After these changes the test can rely on
wrapper.emitted('update:open') to observe the stub emission.
---
Nitpick comments:
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Around line 23-24: Remove the redundant v-show on the `.modal-content` element
since the outer `<div class="modal" role="dialog" aria-modal="true"
v-if="open">` already controls mounting; locate the `<div class="modal-content"
v-show="open">` (in AvailabilityInfoModal template) and delete the v-show="open"
attribute so the markup becomes `<div class="modal-content">`.
| <div class="modal-header"> | ||
| <h2 class="standard-headline"><slot name="title"/></h2> | ||
| <!-- props direkt verwenden, nicht props.closeAriaLabel --> | ||
| <button class="modal-button-close" :aria-label="closeAriaLabel || 'Dialog schließen'" type="button" @click="$emit('update:open', false; $parent.$emit('update:open', false)"></button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the AvailabilityInfoModal.vue component file
fd "AvailabilityInfoModal.vue" -type fRepository: it-at-m/eappointment
Length of output: 234
🏁 Script executed:
#!/bin/bash
# Search for defineModel, defineEmits, or emit patterns related to update:open
rg "defineModel|defineEmits|emit.*update:open" --type vue --type tsRepository: it-at-m/eappointment
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Examine the test file to confirm the syntax error on line 28
fd "AvailabilityInfoModal.spec.ts" --exec head -n 35 {} \; | tail -n 20Repository: it-at-m/eappointment
Length of output: 1090
🏁 Script executed:
#!/bin/bash
# Find the AvailabilityInfoModal.vue component (not the .spec.ts file)
fd "AvailabilityInfoModal" -type f | grep -v "spec.ts"Repository: it-at-m/eappointment
Length of output: 234
🏁 Script executed:
#!/bin/bash
# Search for defineModel, defineEmits patterns in the entire codebase
rg "defineModel|defineEmits" -lRepository: it-at-m/eappointment
Length of output: 1039
🏁 Script executed:
#!/bin/bash
# Check the test lines 123-130 to see the event test
fd "AvailabilityInfoModal.spec.ts" --exec sed -n '118,130p' {}Repository: it-at-m/eappointment
Length of output: 522
🏁 Script executed:
#!/bin/bash
# Inspect the actual AvailabilityInfoModal.vue component
cat -n zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue | head -n 80Repository: it-at-m/eappointment
Length of output: 1608
🏁 Script executed:
#!/bin/bash
# Search for how the component handles update:open and MucModal
rg -A 5 -B 5 "update:open|MucModal" zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vueRepository: it-at-m/eappointment
Length of output: 968
Syntax error in @click handler will crash the template compiler, and $parent.$emit is an anti-pattern
The attribute value @click="$emit('update:open', false; $parent.$emit('update:open', false))" embeds a ; inside a function-call argument list — that is invalid JavaScript. Vue's template compiler wraps the expression in an arrow function, so this generates a SyntaxError at runtime and the entire stub (and therefore every test) will fail.
Beyond the syntax issue, reaching into $parent to fire events is a Vue 3 anti-pattern: it creates hidden coupling between the stub and the component under test and will silently break whenever the component tree changes.
The correct stub pattern is to emit update:open from the stub itself:
- <button class="modal-button-close" :aria-label="closeAriaLabel || 'Dialog schließen'" type="button" `@click`="$emit('update:open', false; $parent.$emit('update:open', false)"></button>
+ <button class="modal-button-close" :aria-label="closeAriaLabel || 'Dialog schließen'" type="button" `@click`="$emit('update:open', false)"></button>Additionally, the .modal-content div (line 23-24) has a redundant v-show="open". Since the parent .modal already uses v-if="open", the entire child tree is removed when open=false, making the v-show directive unreachable dead code.
- <div class="modal-content" v-show="open">
+ <div class="modal-content">wrapper.emitted('update:open') (line 127) will then correctly capture emissions from the stub.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button class="modal-button-close" :aria-label="closeAriaLabel || 'Dialog schließen'" type="button" @click="$emit('update:open', false; $parent.$emit('update:open', false)"></button> | |
| MucModal: { | |
| props: ['open', 'closeAriaLabel', 'dialogAriaLabel'], | |
| emits: ['update:open'], | |
| template: ` | |
| <div class="modal" role="dialog" aria-modal="true" v-if="open"> | |
| <div class="modal-content"> | |
| <div class="modal-header"> | |
| <h2 class="standard-headline"><slot name="title"/></h2> | |
| <button class="modal-button-close" :aria-label="closeAriaLabel || 'Dialog schließen'" type="button" `@click`="$emit('update:open', false)"></button> | |
| </div> | |
| <div class="modal-body"><slot name="body"/></div> | |
| <div class="modal-footer"><slot name="footer"/></div> | |
| </div> | |
| <div class="modal-backdrop"></div> | |
| </div> | |
| `, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`
at line 28, Fix the broken `@click` expression and remove the $parent
anti-pattern: update the button in the stub (the element using `@click` that
currently reads "$emit('update:open', false; $parent.$emit('update:open',
false)") to simply emit the event itself (call $emit('update:open', false)) so
there's no semicolon-in-argument syntax error and no $parent access; also remove
the redundant v-show="open" from the .modal-content div since the parent .modal
uses v-if="open". After these changes the test can rely on
wrapper.emitted('update:open') to observe the stub emission.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Line 24: The v-show="open" on the .modal-content element is redundant because
the parent .modal already uses v-if="open", so remove the v-show="open"
attribute from the .modal-content element (look for the .modal and
.modal-content elements in the AvailabilityInfoModal component/test fixture) to
clean up dead code and avoid duplicate visibility directives.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue`:
- Around line 2-6: The `@close` handler currently forwards the native CloseEvent
via emit('update:open', $event) which violates the component's emit type (e:
"update:open", value: boolean). Replace that usage so the muc-modal `@close`
handler emits a boolean (use emit('update:open', false)) to match the emit
signature and the footer button behavior; update the handler that references
emit('update:open', $event) to emit('update:open', false).
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Around line 17-35: The test stub for MucModal emits update:open on the close
button but the component under test listens for a close event, so change the
stub or the component to match: either update the stub's emitted event from
"update:open" to "close" (so the component's `@close` handler is triggered) or
change the component to listen for `@update`:open instead of `@close`; also remove
the redundant v-show="open" inside the stub since the outer v-if="open" already
controls rendering. Reference the MucModal stub, the button emitting
$emit('update:open', false), and the component's `@close/`@update:open listener
when making the change.
| <muc-modal | ||
| :open="open" | ||
| @close="emit('update:open', $event)" | ||
| aria-labelledby="modalTitle" | ||
| :closeAriaLabel="closeAriaLabel || 'Dialog schließen'" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd zmscitizenview && find . -name "*.vue" -o -name "*.ts" -o -name "*.js" | grep -i modal | head -20Repository: it-at-m/eappointment
Length of output: 215
🏁 Script executed:
# Check the specific component file being reviewed
cd zmscitizenview && cat -n src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue | head -60Repository: it-at-m/eappointment
Length of output: 1596
🏁 Script executed:
# Look for MucModal imports and dependencies
cd zmscitizenview && rg -n "MucModal" --type vue --type ts -A 3 src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vueRepository: it-at-m/eappointment
Length of output: 91
🏁 Script executed:
# Search for MucModal component definition in the project
cd zmscitizenview && fd -e vue -e ts -i mucmodal -x grep -l ".*" {}Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Check node_modules for `@muenchen` package
cd zmscitizenview && ls -la node_modules/@muenchen/ 2>/dev/null | head -20 || echo "Directory not found"Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Check package.json for dependencies
cd zmscitizenview && cat package.json | grep -A 10 -B 2 "muenchen\|dependency"Repository: it-at-m/eappointment
Length of output: 1188
🏁 Script executed:
cd zmscitizenview && cat -n tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts | head -150Repository: it-at-m/eappointment
Length of output: 6845
🏁 Script executed:
# Search for MucModal in node_modules
cd zmscitizenview && find node_modules/@muenchen -name "*modal*" -o -name "*Modal*" 2>/dev/null | head -10Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Check the muc-patternlab-vue package for component definitions
cd zmscitizenview && find node_modules/@muenchen/muc-patternlab-vue -type f \( -name "*.d.ts" -o -name "*.vue" \) 2>/dev/null | grep -i modalRepository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Look for package.json of muc-patternlab-vue to find entry point
cd zmscitizenview && cat node_modules/@muenchen/muc-patternlab-vue/package.json 2>/dev/null | head -50Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Search for Modal component export or definition in the library
cd zmscitizenview && rg -l "MucModal|defineEmits.*close" node_modules/@muenchen/muc-patternlab-vue 2>/dev/null | head -10Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
cd zmscitizenview && cat -n tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts | sed -n '117,131p'Repository: it-at-m/eappointment
Length of output: 629
🏁 Script executed:
# Check if there are any other MucModal usages in the codebase to see expected patterns
cd zmscitizenview && rg -n "muc-modal\|MucModal" --type vue -B 2 -A 2 src/ | grep -E "(muc-modal|@close|@update:open)" | head -30Repository: it-at-m/eappointment
Length of output: 91
🏁 Script executed:
# Look for documentation or type definitions of MucModal in node_modules
cd zmscitizenview && find node_modules/@muenchen -name "*.d.ts" 2>/dev/null | xargs grep -l "Modal" | head -5Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Try to extract type information about MucModal's emits
cd zmscitizenview && find node_modules/@muenchen/muc-patternlab-vue -type f 2>/dev/null | xargs cat 2>/dev/null | grep -A 5 -B 5 "close\|Modal" | head -60Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
cd zmscitizenview && rg -n "@close|@update:open" --type vue src/components/ | grep -v ".spec.ts" | head -20Repository: it-at-m/eappointment
Length of output: 91
🏁 Script executed:
# Check if there are any other components using muc-modal to see expected pattern
cd zmscitizenview && rg -B 5 -A 5 "muc-modal" src/ --type vue | head -80Repository: it-at-m/eappointment
Length of output: 91
🏁 Script executed:
# Let me search for any actual usages or patterns of muc-modal
cd zmscitizenview && find . -name "*.vue" -type f -exec grep -l "muc-modal" {} \; 2>/dev/nullRepository: it-at-m/eappointment
Length of output: 200
🏁 Script executed:
# Check if node_modules has the actual compiled component
cd zmscitizenview && ls -la node_modules/@muenchen/muc-patternlab-vue/ 2>/dev/nullRepository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Try to find any index file or main export from the library
cd zmscitizenview && cat node_modules/@muenchen/muc-patternlab-vue/package.json 2>/dev/null | grep -E "(main|exports|module|types)"Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Let's check the other file that uses muc-modal to understand the pattern
cd zmscitizenview && cat -n src/components/AppointmentDetail/AppointmentDetailView.vue | grep -A 10 -B 5 "muc-modal"Repository: it-at-m/eappointment
Length of output: 1928
🏁 Script executed:
# Try to find any configuration or type stubs for the library
cd zmscitizenview && find . -name "*.d.ts" -o -name "vite-env.d.ts" | xargs cat 2>/dev/null | grep -i modalRepository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Check if there's a documentation or README that explains the MucModal API
cd zmscitizenview && find . -name "README*" -o -name "*.md" | xargs grep -l -i "muc-modal\|mucmodal" 2>/dev/nullRepository: it-at-m/eappointment
Length of output: 48
🏁 Script executed:
# Let me check if there's any error or failure log in the test
cd zmscitizenview && cat tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts | tail -30Repository: it-at-m/eappointment
Length of output: 1299
🏁 Script executed:
# Check the complete test file to see if there are more tests after line 150
cd zmscitizenview && wc -l tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.tsRepository: it-at-m/eappointment
Length of output: 141
🏁 Script executed:
# Look for any GitHub CI or test configuration that might show actual failures
cd zmscitizenview && find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "test\|spec" 2>/dev/null | head -5Repository: it-at-m/eappointment
Length of output: 46
🏁 Script executed:
# Let me understand the test flow - check if the test is actually testing the `@close` handler
cd zmscitizenview && cat -n tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts | sed -n '118,131p'Repository: it-at-m/eappointment
Length of output: 621
🏁 Script executed:
# Check the stub's emits more carefully - does it have a way to trigger `@close`?
cd zmscitizenview && grep -n "emits\|close\|update:open" tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.tsRepository: it-at-m/eappointment
Length of output: 1007
🏁 Script executed:
# Let me trace through what happens - check if there's any other event emission in the component
cd zmscitizenview && cat -n src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue | sed -n '1,30p'Repository: it-at-m/eappointment
Length of output: 956
🏁 Script executed:
# The key question: when the stub's close button emits 'update:open', does the component catch it?
# Let's check if the component has both `@close` and anything that catches update:open from the modal
cd zmscitizenview && grep -n "update:open\|@close" src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vueRepository: it-at-m/eappointment
Length of output: 200
🏁 Script executed:
# Check if there's a v-model or two-way binding being used with the modal
cd zmscitizenview && cat -n src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue | sed -n '1,10p'Repository: it-at-m/eappointment
Length of output: 367
Use explicit false instead of $event in the @close handler — it violates the emit type contract
The handler @close="emit('update:open', $event)" passes a native CloseEvent object, but the emit definition (line 47) expects a boolean:
(e: "update:open", value: boolean): void;When MucModal fires its close event, $event is a browser Event object, not false. This breaks the type contract. The footer button correctly uses false (line 24), so align the close handler:
Fix
<muc-modal
:open="open"
- `@close`="emit('update:open', $event)"
+ `@close`="emit('update:open', false)"
aria-labelledby="modalTitle"
:closeAriaLabel="closeAriaLabel || 'Dialog schließen'"The test doesn't catch this because the stub only emits update:open (not close), so the real MucModal's close event behavior is untested.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@zmscitizenview/src/components/Appointment/AppointmentSelection/AvailabilityInfoModal.vue`
around lines 2 - 6, The `@close` handler currently forwards the native CloseEvent
via emit('update:open', $event) which violates the component's emit type (e:
"update:open", value: boolean). Replace that usage so the muc-modal `@close`
handler emits a boolean (use emit('update:open', false)) to match the emit
signature and the footer button behavior; update the handler that references
emit('update:open', $event) to emit('update:open', false).
zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts (1)
28-28: Remove noisy implementation reminder commentThe comment
<!-- props direkt verwenden, nicht props.closeAriaLabel -->documents Vue's template prop access — this is standard Vue 3 knowledge and adds no informational value for a reader of the test.🧹 Proposed fix
- <!-- props direkt verwenden, nicht props.closeAriaLabel -->As per coding guidelines: "Don't add obvious noise" and "Don't be redundant" (Comments rules).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts` at line 28, Remove the noisy HTML comment "<!-- props direkt verwenden, nicht props.closeAriaLabel -->" from the AvailabilityInfoModal.spec.ts test file; simply delete that redundant template comment (it references standard Vue prop access) so the test code and template remain unchanged but without the unnecessary reminder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Line 25: The v-show="open" on the .modal-content element is redundant because
the parent .modal already uses v-if="open", so remove the v-show="open"
attribute from the .modal-content element (the element shown as <div
class="modal-content" v-show="open">) to eliminate dead code and avoid
confusion; keep the parent .modal's v-if="open" intact.
- Around line 124-131: In the test stub for the AvailabilityInfoModal (the
component stub where the parent wrapper uses v-if="open"), remove the redundant
v-show="open" attribute from the nested element (since v-if already controls
presence) and delete the German explanatory comment that sits next to the prop
usage; update only the stub markup so it no longer contains the dead v-show and
the unnecessary comment, leaving the prop bindings and emitted behavior
unchanged (look for the stub definition near the top of
AvailabilityInfoModal.spec.ts where v-if="open" and v-show="open" are both
present).
---
Nitpick comments:
In
`@zmscitizenview/tests/unit/Appointment/AppointmentSelection/AvailabilityInfoModal.spec.ts`:
- Line 28: Remove the noisy HTML comment "<!-- props direkt verwenden, nicht
props.closeAriaLabel -->" from the AvailabilityInfoModal.spec.ts test file;
simply delete that redundant template comment (it references standard Vue prop
access) so the test code and template remain unchanged but without the
unnecessary reminder.
1 participant
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Summary by CodeRabbit
Refactor
New Features
Tests
Chores