Skip to content

fix comments in feat/16446-navigation-validation#4094

Merged
walldenfilippa merged 3 commits intofeat/16446-navigation-validationfrom
fix/navigation-validation-comments
Apr 7, 2026
Merged

fix comments in feat/16446-navigation-validation#4094
walldenfilippa merged 3 commits intofeat/16446-navigation-validationfrom
fix/navigation-validation-comments

Conversation

@walldenfilippa
Copy link
Copy Markdown
Contributor

@walldenfilippa walldenfilippa commented Apr 7, 2026

Description

Fix comments:
1.
image
PageGroupMultiple delegates to Page.tsx for each entry, which already handles validation. PageGroupSingle renders its own button directly and therefore did not inherit that logic.
2.
image
Always render the toggle button so the section can be collapsed, and auto-expand it when the user is on the current page, consistent with how PageGroupMultiple behaves.
3.
image
Fixed flaky tests

Related Issue(s)

  • closes #{issue number}

Verification/QA

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* and backport* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

@walldenfilippa walldenfilippa added kind/other Pull requests containing chores/repo structure/other changes backport-ignore This PR is a new feature and should not be cherry-picked onto release branches squad/utforming Issues that belongs to the named squad. labels Apr 7, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b4ef29db-9d67-4ef7-9758-316effd3180d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/navigation-validation-comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@Magnusrm Magnusrm left a comment

Choose a reason for hiding this comment

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

Great work! 🎉
Just a minor nitpick below

Comment on lines +102 to +125
if (isCurrentPage || !currentPageId) {
return;
}

const currentIndex = order.indexOf(currentPageId);
const targetIndex = order.indexOf(page);
if (currentIndex === -1 || targetIndex === -1) {
return;
}

const isForward = targetIndex > currentIndex;
const validationOnNavigation = getPageValidation();

await maybeSaveOnPageChange();

if (isForward && validationOnNavigation) {
const hasValidationErrors = await onPageNavigationValidation(currentPageId, validationOnNavigation);
if (hasValidationErrors) {
return;
}
}

await navigateToPage(page);
onNavigate?.();
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.

Nitpick: I think it would be neater to extract this code into a function instead of directly in the onClick.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

@Magnusrm Magnusrm left a comment

Choose a reason for hiding this comment

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

Awesome! 😁

@walldenfilippa walldenfilippa merged commit 84d3491 into feat/16446-navigation-validation Apr 7, 2026
14 of 16 checks passed
@walldenfilippa walldenfilippa deleted the fix/navigation-validation-comments branch April 7, 2026 12:00
walldenfilippa added a commit that referenced this pull request Apr 7, 2026
* #17511 + #17512 (#3977)

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>

* Override validation on navigationbuttons

* minor clean up

* added override validation on the navigationBar component

* added ovverride customButton + configured the validateOnNext/Forward and validateOnPrevious/Backward with preventNavigation prop for validationOnNavigation

* Feat/prevent-navigation (#3990)

* add basic logic for preventing navigation

* change wording

* added PageValidation on layoutSets and layoutSettings (#3998)

* added PageValidation on layoutSets and layoutSettings

---------

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>

* Feat/refactor pagevalidation navigation (#4005)

* refactor: navigation validation backward and improve page validation hooks

* refactor preventing navigation to use simplified config

---------

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>
Co-authored-by: Magnus Revheim Martinsen <mrmartinsen.96@gmail.com>

* added prop expandedByDefault to groups for sidenavigation (#4006)

* added prop expandedByDefault to groups for sidenavigation

* added prop expandedByDefault to subform sidenavigation

---------

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>

* Updated the validation hierarchy for the navigation rules.

* fix prevent navigation

* feat: enhance navigation validation by integrating validation checks (#4027)

* feat: enhance navigation validation by integrating validation checks in navigation components

* removed one hook and added useGetNavigationIsPrevented on pageGroup aswell

* temporary notes

* remove temporary note

* update tests

---------

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>

* merge main into feat/16446-navigation-validation

* changed ovverride level direction on validation on navigation and moved validationOnNavigation to GlobalPageSettings (#4064)

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>

* fix comments in feat/16446-navigation-validation (#4094)

* fix comments in feat/16446-navigation-validation

* fixed tests in NavigationButtons tests

* refactor: implement navigation with validation logic in Page and PageGroup components

---------

Co-authored-by: walldenfilippa <filippa.walden@digdir.no>

---------

Co-authored-by: Filippa Wallden <143729834+walldenfilippa@users.noreply.github.com>
Co-authored-by: walldenfilippa <filippa.walden@digdir.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-ignore This PR is a new feature and should not be cherry-picked onto release branches kind/other Pull requests containing chores/repo structure/other changes squad/utforming Issues that belongs to the named squad.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants