From 6e77cdd797b16ff8bc3999aaa0347c87ddc9abf5 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Tue, 8 Sep 2026 15:18:10 +0200 Subject: [PATCH 1/2] fix(dialog): stick the action bars to the bottom of the panel The bars were positioned by the panel's own bottom padding, which left them floating above the content when the form was shorter than the dialog. Signed-off-by: skjnldsv --- lib/dialog/SharingDialog.vue | 3 --- lib/dialog/components/SharePanel.vue | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dialog/SharingDialog.vue b/lib/dialog/SharingDialog.vue index 850ec21..b2b9076 100644 --- a/lib/dialog/SharingDialog.vue +++ b/lib/dialog/SharingDialog.vue @@ -216,9 +216,6 @@ onMounted(async () => { // changes (switching tabs, adding recipients, revealing toggles). min-height: min(320px, 50vh); overflow-y: auto; - // Match the dialog's inline padding at the bottom (its content has none), - // so the form does not sit flush against the edge. - padding-block-end: calc(var(--default-grid-baseline) * 3); } &__loading, diff --git a/lib/dialog/components/SharePanel.vue b/lib/dialog/components/SharePanel.vue index ec267a2..e618ccc 100644 --- a/lib/dialog/components/SharePanel.vue +++ b/lib/dialog/components/SharePanel.vue @@ -469,7 +469,10 @@ form.share-panel { z-index: 2; background-color: var(--color-main-background); border-block-start: 1px solid var(--color-border); + // Match the form's vertical padding so the bar does not float above the content. padding-block: calc(var(--default-grid-baseline) * 3); + // Stick it to the bottom of the form + margin-block-start: auto; // Cancel the form's bottom padding so the bar sits flush at the bottom. margin-block-end: calc(var(--default-grid-baseline) * -3); } From d206a0104a84e79fcdfe3e506ad58981737df55d Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Tue, 8 Sep 2026 15:18:32 +0200 Subject: [PATCH 2/2] fix(dialog): drop the toggle from free text properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A note has an obvious empty state, so the switch beside it only added a step: the field was disabled until it was flipped on, and flipping it off was the way to remove a note. Render free text without a toggle instead, always editable, and let an empty field mean the property is unset — which is what a required note already did. Clearing the text is now the only way to unset such a property, so a pending edit is flushed when the field goes away; the debounce would otherwise keep the old note when the dialog closes right after typing. Properties that are not free text keep their toggle: for a password or an expiry, "on with no value yet" is a state worth expressing, and switching one off says something the empty value alone does not. Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: skjnldsv --- .../components/InlineToggleField.spec.ts | 7 +--- lib/dialog/components/InlineToggleField.vue | 8 ----- lib/dialog/components/PropertyField.spec.ts | 28 +++++++++++++++ lib/dialog/components/PropertyField.vue | 6 +++- lib/dialog/components/SharePanel.spec.ts | 35 ++++++++++++++++++- lib/dialog/components/SharePanel.vue | 6 ++-- 6 files changed, 70 insertions(+), 20 deletions(-) diff --git a/lib/dialog/components/InlineToggleField.spec.ts b/lib/dialog/components/InlineToggleField.spec.ts index 4f13e9b..b74c924 100644 --- a/lib/dialog/components/InlineToggleField.spec.ts +++ b/lib/dialog/components/InlineToggleField.spec.ts @@ -18,7 +18,7 @@ const TOGGLE_INPUT = '.inline-toggle-field__toggle input' * * @param props Component props */ -function mountField(props: { modelValue: boolean, label?: string, longText?: boolean }): VueWrapper { +function mountField(props: { modelValue: boolean, label?: string }): VueWrapper { return mount(InlineToggleField, { props: { label: 'Note', ...props }, slots: { @@ -102,11 +102,6 @@ describe('InlineToggleField', () => { await expect(wrapper.find(TOGGLE_INPUT).setValue(true)).resolves.not.toThrow() }) - it('adds the long-text class on the toggle when longText is set', () => { - const wrapper = mountField({ modelValue: true, longText: true }) - expect(wrapper.find('.inline-toggle-field__toggle--long-text').exists()).toBe(true) - }) - it('exposes the group aria-label and wires the slot input id', () => { const wrapper = mountField({ modelValue: true, label: 'Expiration' }) expect(wrapper.find('[role="group"]').attributes('aria-label')).toBe('Expiration') diff --git a/lib/dialog/components/InlineToggleField.vue b/lib/dialog/components/InlineToggleField.vue index 3f295b9..7012c92 100644 --- a/lib/dialog/components/InlineToggleField.vue +++ b/lib/dialog/components/InlineToggleField.vue @@ -23,7 +23,6 @@ :aria-controls="inputId" :aria-label="label" class="inline-toggle-field__toggle" - :class="{ 'inline-toggle-field__toggle--long-text': longText }" type="switch" @update:modelValue="onToggleEnabled" /> @@ -39,8 +38,6 @@ const modelValue = defineModel({ default: false }) defineProps<{ /** Accessible label of the field group and its toggle */ label: string - /** Align the toggle to the first line of a multi-line field (e.g. textarea) */ - longText?: boolean }>() const inputId = `property-input-${Math.random().toString(36).slice(2, 9)}` @@ -87,11 +84,6 @@ async function onToggleEnabled(enabled: boolean) { &__toggle { height: var(--default-clickable-area); - - &--long-text { - align-self: flex-start; - margin-top: 6px; - } } } diff --git a/lib/dialog/components/PropertyField.spec.ts b/lib/dialog/components/PropertyField.spec.ts index a105252..3d2393b 100644 --- a/lib/dialog/components/PropertyField.spec.ts +++ b/lib/dialog/components/PropertyField.spec.ts @@ -164,6 +164,34 @@ describe('PropertyField persistence', () => { expect(mockedUpdate).toHaveBeenCalledWith(PROPERTY_CLASS, 'Hey') }) + it('unsets the property when the field is cleared', async () => { + const wrapper = mount(PropertyField, { + props: { property: property({ value: 'Old note' }), share: shareMock, modelValue: 'Old note' }, + attachTo: document.body, + }) + const input = wrapper.find('input[type="text"]') + vi.spyOn(input.element as HTMLInputElement, 'checkValidity').mockReturnValue(true) + + // There is no toggle to switch off: clearing the text is how a property + // without one gets unset. + await input.setValue('') + await vi.advanceTimersByTimeAsync(500) + + expect(mockedUpdate).toHaveBeenCalledWith(PROPERTY_CLASS, null) + }) + + it('dispatches a pending edit when the field goes away', async () => { + const wrapper = mountField() + const input = wrapper.find('input[type="text"]') + vi.spyOn(input.element as HTMLInputElement, 'checkValidity').mockReturnValue(true) + + await input.setValue('Hello') + // Closing the dialog right after typing must not drop the edit. + wrapper.unmount() + + expect(mockedUpdate).toHaveBeenCalledWith(PROPERTY_CLASS, 'Hello') + }) + it('skips the request and reports validity when the input is invalid', async () => { const wrapper = mountField() const input = wrapper.find('input[type="text"]') diff --git a/lib/dialog/components/PropertyField.vue b/lib/dialog/components/PropertyField.vue index 521f2c8..6c72620 100644 --- a/lib/dialog/components/PropertyField.vue +++ b/lib/dialog/components/PropertyField.vue @@ -108,7 +108,7 @@ import type { SharingProperty } from '../types/api.ts' import IconInformationOutline from '@mdi/svg/svg/information-outline.svg?raw' import debounce from 'debounce' -import { nextTick, ref, useTemplateRef } from 'vue' +import { nextTick, onBeforeUnmount, ref, useTemplateRef } from 'vue' import NcDateTimePickerNative from '@nextcloud/vue/components/NcDateTimePickerNative' import NcFormBox from '@nextcloud/vue/components/NcFormBox' import NcFormBoxSwitch from '@nextcloud/vue/components/NcFormBoxSwitch' @@ -161,6 +161,10 @@ function parseISODate(value: string | null | undefined): Date | undefined { const debouncedPersist = debounce(persistValue, 500) +// A field without a toggle is unset by clearing it, so a pending edit must not +// be lost when the dialog closes right after typing. +onBeforeUnmount(() => debouncedPersist.flush()) + /** * Update the local value immediately and schedule a debounced persist. * Converts component-native values (boolean, Date) to string for the API. diff --git a/lib/dialog/components/SharePanel.spec.ts b/lib/dialog/components/SharePanel.spec.ts index acce3c9..a3fb41f 100644 --- a/lib/dialog/components/SharePanel.spec.ts +++ b/lib/dialog/components/SharePanel.spec.ts @@ -8,7 +8,7 @@ import type { SharingShare } from '../types/api.ts' import { flushPromises, shallowMount } from '@vue/test-utils' import { beforeEach, describe, expect, it, vi } from 'vitest' import SharePanel from './SharePanel.vue' -import { PROPERTY_EXPIRATION, PROPERTY_PASSWORD, RECIPIENT_TYPE_TOKEN, RECIPIENT_TYPE_USER, SOURCE_TYPE_NODE } from '../constants.ts' +import { PROPERTY_EXPIRATION, PROPERTY_NOTE, PROPERTY_PASSWORD, RECIPIENT_TYPE_TOKEN, RECIPIENT_TYPE_USER, SOURCE_TYPE_NODE } from '../constants.ts' import { ShareDialogTab } from '../types/ui.ts' const PRESET_VIEW = 'preset-view' @@ -278,6 +278,39 @@ describe('SharePanel submit', () => { }) }) +describe('SharePanel properties', () => { + /** A note property: free text, long enough to render as a textarea. */ + const note = (value: string | null) => ({ + class: PROPERTY_NOTE, + display_name: 'Note to recipients', + hint: null, + priority: 9, + required: false, + max_length: 1000, + value, + type: 'string' as const, + }) + + it('offers free text without a toggle, editable while empty', () => { + const { wrapper } = mountPanel(schema({ properties: [note(null)] })) + + // Nothing to switch on first: an empty field already says "no note". + expect(wrapper.findComponent({ name: 'InlineToggleField' }).exists()).toBe(false) + const field = wrapper.findComponent({ name: 'PropertyField' }) + expect(field.exists()).toBe(true) + expect(field.props('disabled')).toBeFalsy() + }) + + it('keeps the toggle for properties that are not free text', () => { + const { wrapper } = mountPanel(schema({ + properties: [{ class: PROPERTY_PASSWORD, display_name: 'Password', hint: null, priority: 6, required: false, value: null, type: 'password' }], + }), { inSettings: true }) + + // A password is meaningfully "on with no value yet", so it keeps its switch. + expect(wrapper.findComponent({ name: 'InlineToggleField' }).exists()).toBe(true) + }) +}) + describe('SharePanel notes', () => { it('summarizes expiration and password when both are set', () => { const data = schema({ diff --git a/lib/dialog/components/SharePanel.vue b/lib/dialog/components/SharePanel.vue index e618ccc..f6c2e82 100644 --- a/lib/dialog/components/SharePanel.vue +++ b/lib/dialog/components/SharePanel.vue @@ -56,9 +56,8 @@