feat: add NcFormGroup #7689

ShGKme · 2025-10-20T11:45:01Z

☑️ Resolves

Fix Add <NcFieldset> component #7122
Closes feat(NcFieldset): new component to structure settings #7352
Differences from the initial plan:
- No special "nesting" support — it was decided to use specific section components for large form sections like <NcAppSettingsSection>
- No content style changing — it will be handled by a special NcFormBox component

The current solution has some TODOs.

🖼️ Screenshots

🏁 Checklist

⛑️ Tests are included or are not applicable
📘 Component documentation has been extended, updated or is not applicable
2️⃣ Backport to stable8 for maintained Vue 2 version or not applicable

ShGKme · 2025-10-20T11:48:22Z

@kra-mo Currently the heading (legend, description) has 12px inline padding as per design. To be inline with elements having 8px border radius. So we can call it "Radius + 1 grid baseline".

Is that correct and what is intended by the design?

In thi case, we'll also adjust the field label paddings.

susnux · 2025-10-20T19:58:07Z

I think it would be better to make the prop called either name or label and not introduce a third variant for the label of the element here ("legend")

ShGKme · 2025-10-21T00:51:59Z

name has a different meaning. We have it in many places after the change (going from title) in v8.0.0, and it was quite consistent. But later new components (such as NcDialogButton, NcRadioGroup) were introduced with a new prop already (label).

Unlike name, label may work here by the meaning (it labels an element with the group role).

But I wouldn't call the legend a "third variant". It represents the <legend> element of the <fieldset>.

Open for others' opinions.

kra-mo · 2025-10-21T07:57:57Z

Nice, I'm really happy to see this :)

@kra-mo Currently the heading (legend, description) has 12px inline padding as per design. To be inline with elements having 8px border radius. So we can call it "Radius + 1 grid baseline".

Is that correct and what is intended by the design?

In thi case, we'll also adjust the field label paddings.

Do you mean to adjust the input fields' leading padding to match? Then yes, it should just be aligned.

Mockup	Implementation

No special "nesting" support — it was decided to use specific section components for large form sections like

Yes, I thought about this too, it makes total sense :)

No content style changing — it will be handled by a special NcFormGroup component

Just so I understand: where would the FormGroup sit in the tree relative to individual fields and the Fieldset itself?

PS: Not to encroach on your territory, but shouldn't the S in FieldSet be capital? Just because the XML tag comes from two words. But please discard me if you think lowercase is more in line with naming standards :)

src/components/NcFieldset/NcFieldset.vue

ShGKme · 2025-10-21T10:14:01Z

Do you mean to adjust the input fields' leading padding to match? Then yes, it should just be aligned.

No, the question wasn't "should they be aligned" but is the margin supposed to be border radius + 1 grid baseline. I can see that it is 8 + 4 = 12px from the .png for the Nextcloud 33 theme, but I don't know that was the idea behind the value to adjust it to the previus versions.

kra-mo · 2025-10-21T10:17:18Z

No, the question wasn't "should they be aligned" but is the margin supposed to be border radius + 1 grid baseline. I can see that it is 8 + 4 = 12px from the .png for the Nextcloud 33 theme, but I don't know that was the idea behind the value to adjust it to the previus versions.

Okay, yes. 12px came from "it looked the best for fields both with and without a description" :)

Border radius + 1 grid baseline makes sense for me to describe it if you think it does as well.

kra-mo · 2025-10-21T10:19:26Z

Then yes, it should just be aligned.

And what I was mainly trying to say here was just that "it is not my territory, as long as it is aligned, I am happy" :)

ShGKme · 2025-10-21T10:28:06Z

/backport to stable8

susnux · 2025-10-21T10:29:13Z

I think name is not really good as it confuses with technical name within forms.

But label is basically what this does, it labels the element in an accessible way (possible synonym would be 'caption').
Especially thinking for a11y its common to use the term label as this also will set the accessible name of the fieldset.

I mean yes its technically a legend but this is only down in the implementation, the user only sees this as the label / name of the element, similar to other elements.
So from the API point for me it makes sense to have consistent names for similar use cases, so if I - as a library user - use a component I did not use before I already know what kind of props I can expect :)

susnux · 2025-10-21T10:43:53Z

src/components/NcFieldset/NcFieldset.vue

+	legend = undefined,
+	description = undefined,
+	hideLegend = false,
+	hideDescription = false,
+	noGap = false,
+} = defineProps<{
+	/**
+	 * Fieldset legend. #legend slot can be used for custom legend content
+	 */
+	legend?: string
+	/**
+	 * Optional fieldset description. #description slot can be used for custom description content
+	 */
+	description?: string
+	/**
+	 * Hide the legend visually but keep it accessible for screen readers
+	 */
+	hideLegend?: boolean


Suggested change

legend = undefined,

description = undefined,

hideLegend = false,

hideDescription = false,

noGap = false,

} = defineProps<{

/**

* Fieldset legend. #legend slot can be used for custom legend content

*/

legend?: string

/**

* Optional fieldset description. #description slot can be used for custom description content

*/

description?: string

/**

* Hide the legend visually but keep it accessible for screen readers

*/

hideLegend?: boolean

label = undefined,

description = undefined,

hideLabel = false,

hideDescription = false,

noGap = false,

} = defineProps<{

/**

* Label of the field set. `#label` slot can be used for custom formatted label content.

*/

label?: string

/**

* Optional fieldset description. #description slot can be used for custom description content

*/

description?: string

/**

* Hide the label visually but keep it accessible for screen readers

*/

hideLabel?: boolean

susnux · 2025-10-21T10:44:14Z

src/components/NcFormGroup/NcFormGroup.vue

+	/**
+	 * Disable default fieldset content gap between content elements
+	 */
+	noGap?: boolean


Whats the use case?

General good practice for UI components — if a component has not only its own styles but also its position/content (its margin, slots styles) - it should be optional.

It could be a list of checkboxes without a gap, or some custom set of elements.

Alternative - ask to wrap content in a div.

susnux · 2025-10-21T10:44:39Z

src/components/NcFieldset/NcFieldset.vue

+	/**
+	 * Custom legend content
+	 */
+	legend?: Slot


See other comment

Suggested change

/**

* Custom legend content

*/

legend?: Slot

/**

* Custom label content

*/

label?: Slot

susnux · 2025-10-21T10:47:19Z

src/components/NcFieldset/NcFieldset.vue

+			<slot name="legend">
+				{{ legend || '⚠️ Missing legend' }}


See above

Suggested change

<slot name="legend">

{{ legend || '⚠️ Missing legend' }}

<slot name="label">

{{ label || '⚠️ Missing label' }}

susnux · 2025-10-21T10:47:28Z

src/components/NcFieldset/NcFieldset.vue

+	<fieldset
+		:class="[$style.fieldset, { [$style.fieldset_noGap]: noGap }]"
+		:aria-describedby="getDescriptionId()">
+		<legend :id="labelId" :class="[$style.fieldset__legend, { 'hidden-visually': hideLegend }]">


Suggested change

<legend :id="labelId" :class="[$style.fieldset__legend, { 'hidden-visually': hideLegend }]">

<legend :id="labelId" :class="[$style.fieldset__legend, { 'hidden-visually': hideLabel }]">

ShGKme · 2025-10-21T11:29:23Z

I mean yes its technically a legend but this is only down in the implementation, the user only sees this as the label / name of the element, similar to other elements.

But the prop isn't for the end user, it is for the developers, and it represents <legend> element.

If we consider <fieldset> + <legend> only an internal detail that should not be reflected in the naming, do you also mean that we should rename the component itself? (Because <fieldset> is an internal implementation, and it could be just <div role="group" aria-labelledby="ID">)

susnux · 2025-10-21T12:09:20Z

But the prop isn't for the end user, it is for the developers, and it represents element.

End user is the developer in that case and API wise it makes sense to have a consistent way of naming instead of forcing the developer to think about that component is used behind and what special cases it has.

We always use label for this where name is not appropriate, so I think we should stick with it instead of add a new one just because the underlying HTML tag uses a different name.
So at least our API is consistent when HTML is not.

ShGKme · 2025-10-21T12:15:15Z

But the prop isn't for the end user, it is for the developers, and it represents element.

End user is the developer in that case and API wise it makes sense to have a consistent way of naming instead of forcing the developer to think about that component is used behind and what special cases it has.

We always use label for this where name is not appropriate, so I think we should stick with it instead of add a new one just because the underlying HTML tag uses a different name. So at least our API is consistent when HTML is not.

What about the second paragraph of the quoted comment? :)

susnux · 2025-10-21T12:37:52Z

What about the second paragraph of the quoted comment? :)

You mean:

(Because
is an internal implementation, and it could be just
)

Do you have a better name? I think Fieldset is quite easy to understand as a set of fields.

ShGKme · 2025-10-21T14:15:27Z

Do you have a better name?

I used NcFieldset + :legend as the names for <fieldset> + <legend>.

If we consider :legend a bad name because it is semantically a label internally implemented via <legend>, then a component can be called <NcFormGroup> because it is semantically a group internally implemented via <fieldset>.

susnux · 2025-10-21T14:33:22Z

If we consider :legend a bad name because it is semantically a label internally implemented via , then a component can be called because it is semantically a group internally implemented via
.

As said its more about consistency, as a developer I expect API to be consistent. If other naming-elements are called label I also expect label here and not have to think 'ah yes in HTMl they decided to go with a new legend element instead of label' just to know i have to use the legend prop just for this component.

The name of the component I do not really case as long as the use case is clear.

susnux · 2025-10-21T14:36:01Z

The name of the component I do not really case as long as the use case is clear.

also for #7690 : I am not sure the naming is good. Because its not really a group as a group would semantically require a name. I expected this component here (NcFieldset) to be called NcFormGroup as discussed earlier and the other component NcFormBox or similar.

I do not care about the actual name of the component, but they should be connected and represent their use case :)

ShGKme · 2025-10-21T14:53:05Z

As said its more about consistency, as a developer I expect API to be consistent

Me too. But "consistency" doesn't mean that the same world should be used everywhere. name is a great example, where we have the same "consistent" prop title when it means different things.

Here for consistency the prop was named legend for the <legend>'s value of the <fieldset> in the NcFieldset component.

ShGKme · 2025-10-21T16:16:47Z

Rebased to have clean CI, no changes in the code

codecov · 2025-10-21T16:18:46Z

Codecov Report

❌ Patch coverage is 3.57143% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.32%. Comparing base (d98dac4) to head (45c11ec).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/components/NcFormGroup/NcFormGroup.vue	4.00%	24 Missing ⚠️
src/components/NcFormGroup/index.ts	0.00%	1 Missing and 1 partial ⚠️
src/components/index.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7689      +/-   ##
==========================================
- Coverage   43.38%   43.32%   -0.07%     
==========================================
  Files         277      279       +2     
  Lines       17990    18018      +28     
  Branches     1009     1011       +2     
==========================================
+ Hits         7805     7806       +1     
- Misses      10134    10160      +26     
- Partials       51       52       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

susnux · 2025-10-21T17:21:24Z

Here for consistency the prop was named legend for the 's value of the
in the NcFieldset component.

But its basically a label there is no reason to have legend except from trying to keep it in sync with HTML which makes no sense because in contrast to name vs title there is no conflict with label.
The legend labels the fieldset so we in a custom component can use a prop name that is consistent with our API instead of this one special case in HTML.

ShGKme · 2025-10-21T17:40:05Z

is no reason to have legend

A reason — to not give one thing a name of another thing when having a component directly representing an existing element.

Otherwise let's not call it a fieldset to not confuse with an existing element.

susnux · 2025-10-22T12:39:22Z

Otherwise let's not call it a fieldset to not confuse with an existing element.

I have no problem with a different component name, not as this is a quite generic component what do you think?
Because I think that the best alternative name would be NcFormGroup but how to call the other component then?

ShGKme · 2025-10-23T12:42:59Z

not as this is a quite generic component what do you think

I didn't understand what you mean here...

Because I think that the best alternative name would be NcFormGroup but how to call the other component then?

As was proposed - NcFormBox, because it is visually a "box", quite a common term in UI kits.

GVodyanov · 2025-10-23T16:43:20Z

src/components/NcFormGroup/NcFormGroup.vue

+		:aria-describedby="getDescriptionId()">
+		<legend :id="labelId" :class="[$style.fieldset__legend, { 'hidden-visually': hideLabel }]">
+			<slot name="label">
+				{{ label || '⚠️ Missing label' }}


Do we want to always have a label? I can imagine a use-case like having a Fieldset inside a Modal which already has a title. I feel something might eventually come up where you would want to just not have a label, and having it show "Missing label" would be uncomfortable.

If we want to always have a standard label as a design decision though, I could understand it.

I want to ask again:

Just so I understand: where would the FormGroup sit in the tree relative to individual fields and the Fieldset itself?

But from what I understand right now, from a design POV, there is definitely a case where there is no label, including in the mockups.

Do we want to always have a label?

If its a fieldset we must. Its obligatory for wcag. If its only visually then its the form box not a real group.

GVodyanov

I'll leave design decisions to the team, but code-wise this looks really good!

susnux · 2025-10-23T23:59:28Z

As was proposed - NcFormBox, because it is visually a "box", quite a common term in UI kits.

As said I am 💯 for that name. Meaning form group and form box.
But maybe even if its not 100% accurate also call it form group button, not form box button?

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>

ShGKme · 2025-10-29T14:12:53Z

Component was renamed NcFieldset -> NcFormGroup
Prop was renamed legend -> label, hideLegend -> hideLabel
Auto-margin between NcFormGroup has been removed

kra-mo

Looking good :)

GVodyanov · 2025-11-03T09:48:52Z

/backport to stable8

ShGKme added this to the 9.1.0 milestone Oct 20, 2025

ShGKme requested review from kra-mo and susnux October 20, 2025 11:45

ShGKme self-assigned this Oct 20, 2025

ShGKme added enhancement New feature or request 3. to review Waiting for reviews labels Oct 20, 2025

ShGKme force-pushed the feat/NcFieldset branch from a3b5a89 to 60a4400 Compare October 20, 2025 12:57

kra-mo reviewed Oct 21, 2025

View reviewed changes

src/components/NcFieldset/NcFieldset.vue Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

ChristophWurst requested a review from GVodyanov October 21, 2025 10:18

backportbot bot added the backport-request label Oct 21, 2025

susnux reviewed Oct 21, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

susnux mentioned this pull request Oct 21, 2025

feat: add NcFormBox and adjust NcRadioGroup to NcFormbox and NcFormGroup #7690

Merged

3 tasks

ShGKme force-pushed the feat/NcFieldset branch from b51138a to 5dae268 Compare October 21, 2025 16:16

GVodyanov reviewed Oct 23, 2025

View reviewed changes

GVodyanov approved these changes Oct 23, 2025

View reviewed changes

feat: add NcFormGroup

45c11ec

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>

ShGKme force-pushed the feat/NcFieldset branch from 5dae268 to 45c11ec Compare October 29, 2025 14:07

ShGKme changed the title ~~feat: add NcFieldset~~ feat: add NcFormGroup Oct 29, 2025

susnux approved these changes Oct 29, 2025

View reviewed changes

kra-mo approved these changes Oct 30, 2025

View reviewed changes

susnux merged commit d4b7304 into main Oct 30, 2025
25 of 27 checks passed

susnux deleted the feat/NcFieldset branch October 30, 2025 20:50

backportbot bot mentioned this pull request Nov 3, 2025

[stable8] feat: add NcFormGroup #7762

Merged

2 tasks

backportbot bot removed the backport-request label Nov 3, 2025

ShGKme mentioned this pull request Nov 3, 2025

fix(NcFormGroup): remove margin when no label/description #7764

Merged

susnux modified the milestones: 9.1.0, 9.2.0 Nov 4, 2025

ShGKme mentioned this pull request Nov 7, 2025

release: v9.2.0 #7823

Merged

	<legend :id="labelId" :class="[$style.fieldset__legend, { 'hidden-visually': hideLegend }]">
	<legend :id="labelId" :class="[$style.fieldset__legend, { 'hidden-visually': hideLabel }]">

feat: add NcFormGroup #7689

feat: add NcFormGroup #7689

Uh oh!

Conversation

ShGKme commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☑️ Resolves

🖼️ Screenshots

🏁 Checklist

Uh oh!

ShGKme commented Oct 20, 2025

Uh oh!

susnux commented Oct 20, 2025

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

kra-mo commented Oct 21, 2025

Uh oh!

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

kra-mo commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

kra-mo commented Oct 21, 2025

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

susnux commented Oct 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

ShGKme commented Oct 21, 2025

Uh oh!

This comment was marked as resolved.

susnux commented Oct 21, 2025

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

susnux commented Oct 21, 2025

Uh oh!

ShGKme commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

susnux commented Oct 21, 2025

Uh oh!

susnux commented Oct 21, 2025

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

codecov bot commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

susnux commented Oct 21, 2025

Uh oh!

ShGKme commented Oct 21, 2025

Uh oh!

susnux commented Oct 22, 2025

Uh oh!

ShGKme commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kra-mo Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ShGKme commented Oct 20, 2025 •

edited

Loading

kra-mo commented Oct 21, 2025 •

edited

Loading

ShGKme commented Oct 21, 2025 •

edited

Loading

codecov bot commented Oct 21, 2025 •

edited

Loading

kra-mo Oct 23, 2025 •

edited

Loading

susnux commented Oct 23, 2025 •

edited

Loading