Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pf4): fix bug in wizard navigation toolbar #1414

Merged

Conversation

lucasgarfield
Copy link
Contributor

In Patternfly 4, the WizardNavItem component step prop is "the step passed into the onNavItemClick callback".

http://v4-archive.patternfly.org/v4/components/wizard#wizardnavitem

In Patternfly 5, onNavItemClick no longer exists. Instead, there is an onClick callback which expects the index of the step to be navigated to.

https://www.patternfly.org/components/wizard/#wizardnavitem

However, filtering the navSchema like so: .filter((field) => field.primary) results in substeps with field.primary===false being dropped. Practically, this means the 2nd substep and those following it. Therefore, the index provided by .map((step, ind) => ... is incorrect.

The correct index can still be found from step.index.

We first encountered a bug related to this while upgrading our app to PF5, see the PR here:
osbuild/image-builder-frontend#1365 (comment)

In Patternfly 4, the WizardNavItem component `step` prop is "the step
passed into the onNavItemClick callback".

http://v4-archive.patternfly.org/v4/components/wizard#wizardnavitem

In Patternfly 5, onNavItemClick no longer exists. Instead, there is an
onClick callback which expects the index of the step to be navigated to.

https://www.patternfly.org/components/wizard/#wizardnavitem

However, filtering the navSchema like so: `.filter((field) =>
field.primary)` results in substeps with field.primary===false being
dropped. Practically, this means the 2nd substep and those following it.
Therefore, the index provided by `.map((step, ind) => ...` is incorrect.

The correct index can still be found from `step.index`.
@vercel
Copy link

vercel bot commented Oct 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-forms ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 13, 2023 2:09pm

@DataDrivenFormsBot
Copy link

A new version (fix) will be released: v3.21.7 [DataDrivenFormsBot]

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #1414 (a94e21e) into master (0e94d34) will not change coverage.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1414   +/-   ##
=======================================
  Coverage   94.96%   94.96%           
=======================================
  Files         210      210           
  Lines        3672     3672           
  Branches     1284     1284           
=======================================
  Hits         3487     3487           
  Misses        185      185           
Files Coverage Δ
...-mapper/src/wizard/wizard-components/wizard-nav.js 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Hyperkid123 Hyperkid123 self-requested a review October 16, 2023 12:47
@Hyperkid123 Hyperkid123 merged commit 020cbc8 into data-driven-forms:master Oct 16, 2023
6 checks passed
@rvsia rvsia added the released label Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants