Skip to content

feat(skjemakomponenter): Legge til Valgfritt og Må fylles ut tags - #1033

Open
lisamarimyreneNVE wants to merge 9 commits into
mainfrom
feature(skjemakomponenter)_Legge_til_valgfritt_og_maa_fylles_ut_tag
Open

lisamarimyreneNVE wants to merge 9 commits into
mainfrom
feature(skjemakomponenter)_Legge_til_valgfritt_og_maa_fylles_ut_tag

Conversation

@lisamarimyreneNVE

@lisamarimyreneNVE lisamarimyreneNVE commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Lagt til "Valgfritt" og "Må fylles ut" tags på:

  • nve-textarea
  • nve-radio-group
  • nve-input
  • nve-combobox
  • nve-checkbox-group
  • nve-segment-group

Inkludert oppdatert dokumentasjon på komponent-sidene og validering-avsnittet under "For utviklere". Gjerne dobbelsjekk at det er riktig kilde jeg har hentet fra UU tilsynet i lenken under det sistnevnte avsnittet.

Skisser: https://www.figma.com/design/0eXhyUrUF7fWi1VaphfpEu/04---%E2%9D%96-Komponenter?m=auto&node-id=5686-19689&t=gGIIlX20JosXCa0s-1

Fixes #1018

@github-actions

Copy link
Copy Markdown
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-meadow-0c645bd03-1033.westeurope.5.azurestaticapps.net

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Fixed badge heights allow wrapped label text to overflow and potentially overlap adjacent form content.

1 open finding
What changed in this PR

Adds “Må fylles ut” and “Valgfritt” label tags to six form components using the shared label template.

Changes:

  • Adds optional-field properties and shared tag styling.
  • Adds required and optional tag rendering tests.
  • Updates documentation and validation examples.
File Description
src/​templates/​label.ts Renders and styles validation tags.
src/​components/​nve-textarea/​nve-textarea.test.ts Tests both tags.
src/​components/​nve-textarea/​nve-textarea.component.ts Enables textarea tags.
src/​components/​nve-segment-group/​nve-segment-group.test.ts Tests both tags.
src/​components/​nve-segment-group/​nve-segment-group.component.ts Enables segment-group tags.
src/​components/​nve-radio-group/​nve-radio-group.test.ts Tests both tags.
src/​components/​nve-radio-group/​nve-radio-group.component.ts Enables radio-group tags.
src/​components/​nve-input/​nve-input.test.ts Tests both tags.
src/​components/​nve-input/​nve-input.component.ts Enables input tags.
src/​components/​nve-combobox/​nve-combobox.test.ts Tests both tags.
src/​components/​nve-combobox/​nve-combobox.component.ts Enables combobox tags.
src/​components/​nve-checkbox-group/​nve-checkbox-group.test.ts Tests both tags.
src/​components/​nve-checkbox-group/​nve-checkbox-group.component.ts Enables checkbox-group tags.
doc-site/​introduction/​forDevelopers/​validation.md Explains validation tags.
doc-site/​components/​nve-textarea.md Updates tag and validation examples.
doc-site/​components/​nve-segment-group.md Updates tag and validation examples.
doc-site/​components/​nve-radio-group.md Updates tag and validation examples.
doc-site/​components/​nve-input.md Updates tag and validation examples.
doc-site/​components/​nve-combobox.md Updates tag and validation examples.
doc-site/​components/​nve-checkbox-group.md Updates tag and validation examples.
doc-site/​.vitepress/​theme/​style.css Adds horizontal example layout.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/templates/label.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 08:18
@github-actions

Copy link
Copy Markdown
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-meadow-0c645bd03-1033.westeurope.5.azurestaticapps.net

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The component changes are consistent, and the remaining documentation defects are minor and non-blocking.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Remove stray language label from second example

doc-site/​components/​nve-segment-group.md:137

Both code fences share one CodeExamplePreview, whose textContent.replace('html', '') removes only the first language label (doc-site/.vitepress/theme/components/CodeExamplePreview.vue:8). The second html label appears as stray text in the live example. Keep both examples in one fenced block.

🧠 Review effort: Balanced

@amish1188 amish1188 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ser veldig bra ut :) bra du tok den!
Jeg har bare et par kommentarer.

Comment thread src/templates/label.ts Outdated
Comment thread src/templates/label.ts Outdated
Comment thread src/templates/label.ts Outdated
Comment thread src/templates/label.ts Outdated
<b>To typer validering støttes:</b>
### Valideringstags

Etter retningslinjer fra <a href="https://www.uutilsynet.no/veiledning/skjema/38" target="_blank_">UU Tilsynet</a> er det ikke nødvendig å merke feltene som obligatoriske dersom flertallet av feltene er påkrevd. Brukeren må imidlertid informeres om dette før vedkommende begynner å fylle ut skjemaet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tror vi må kanskje endre litt på disse tekstene, required attributet bør alltid være med, men det er ok å skjule taggen i hvert fall.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Har oppdatert teksten nå, så gjerne se over igjen. Har prøvd å gjøre det mer tydelig.

…ylles_ut_tag' of https://github.com/NVE/Designsystem into feature(skjemakomponenter)_Legge_til_valgfritt_og_maa_fylles_ut_tag
Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:24
@github-actions

Copy link
Copy Markdown
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-meadow-0c645bd03-1033.westeurope.5.azurestaticapps.net

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The remaining feedback concerns non-blocking documentation issues; no blocking functional defects were identified.

1 open finding

🧠 Review effort: Balanced

Comment thread doc-site/components/nve-checkbox-group.md Outdated
Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The shared template breaks native label associations and hides checkbox-group required information from assistive technology.

3 open findings

🧠 Review effort: Balanced

Comment thread src/templates/label.ts
return useLegend
? html`<legend part="label" class="field__legend">${content}</legend>`
: html`<label part="label" class="field__label" for="${id}">${content}</label>`;
: html`<label part="label" class="field__label" id="${id}">${content}</label>`;
Comment thread src/templates/label.ts
const validationTag = required
? options?.hideRequiredTag
? nothing
: html`<span class="field__label__validation field__label__validation--required" aria-hidden="true">${options?.requiredLabel || 'Må fylles ut'}</span>`
Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Reported label-association and required-state accessibility regressions remain unresolved.

3 open findings
Previously missed (1)

In code that hasn't changed since last review

Medium severity Keep both segment groups in a single fenced code block

doc-site/​components/​nve-segment-group.md:141

Both fenced blocks share one CodeExamplePreview. Its renderer (doc-site/.vitepress/theme/components/CodeExamplePreview.vue:8) reads the slot's combined text and removes only the first html language label, leaving the second label visible in the live example. Keep both segment groups in a single fenced block.

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The required checkbox-group example uses an ignored attribute, disabling both the demonstrated tag and required validation.

3 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread doc-site/components/nve-checkbox-group.md Outdated
Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:55
@github-actions

Copy link
Copy Markdown
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-meadow-0c645bd03-1033.westeurope.5.azurestaticapps.net

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The updated label-association tests accept duplicate IDs instead of detecting broken label-to-control relationships.

3 open findings
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Low severity Correct misspelled required attribute

doc-site/​components/​nve-checkbox-group.md:136

requried is not recognized, so this live example neither enables required nor displays the “Må fylles ut” tag. Change it to required.

🧠 Review effort: Balanced

expect(input).not.toBeNull();
expect(input?.id).toBeTruthy();
expect(label?.getAttribute('for')).toBe(input?.id);
expect(label?.getAttribute('id')).toBe(input?.id);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legge til valgfritt/må fylles ut tag på skjemakomponenter

4 participants