Skip to content

feat(pf4): introduce StepTemplate #648

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 14, 2020
Merged

feat(pf4): introduce StepTemplate #648

merged 3 commits into from
Jul 14, 2020

Conversation

rvsia
Copy link
Contributor

@rvsia rvsia commented Jul 14, 2020

  • component allowing to define custom steps
  • also this commit allows to define more props in step definitions

Example:

 {
          name: 'step',
          hasNoBodyPadding: true,
          StepTemplate: ({ formFields }) => <div style={{ width: '100%', height: '100%', background: 'red', padding: 32 }}>{formFields}</div>
          ....
}

image

cc @PanSpagetka

@rvsia rvsia added enhancement New feature or request PF4 PF4 pull request labels Jul 14, 2020
@rvsia rvsia requested a review from Hyperkid123 July 14, 2020 09:55
@rvsia
feat(pf4): introduce StepTemplate
1f46373 
- component allowing to define custom steps
- also this commit allows to define more props in step definitions
@rvsia rvsia force-pushed the pf4StepTemplate branch from 4af954d to 1f46373 Compare July 14, 2020 10:06
@rvsia
Copy link
Contributor Author

rvsia commented Jul 14, 2020

I skipped the one async wizard tests. We have to deal with it later.

#649

@codecov
Copy link

codecov bot commented Jul 14, 2020 •
edited
Loading

Codecov Report

Merging #648 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #648      +/-   ##
==========================================
+ Coverage   92.99%   93.00%   +0.01%     
==========================================
  Files         189      189              
  Lines        3010     3015       +5     
  Branches      989      990       +1     
==========================================
+ Hits         2799     2804       +5     
  Misses        211      211
Impacted Files Coverage Δ
packages/pf4-component-mapper/src/files/wizard.js 100.00% <ø> (ø)
...4-component-mapper/src/files/wizard/wizard-step.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39b7ebe...885dc36. Read the comment docs.

Hyperkid123
Hyperkid123 reviewed Jul 14, 2020
View reviewed changes
packages/pf4-component-mapper/src/files/wizard.d.ts
title?: ReactNode;
formFields: ReactNode;
showTitles?: boolean;
showTitle?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between showTitle and showTitles? Maybe we could use different names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ShowTitles is on wizard, ShowTitle is on step

Copy link
Member

Choose a reason for hiding this comment

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

Maybe rename the showTitle to something like show header? The difference is so tiny that I had to read it 3 times to notice the plural.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't matter. showTitles is set in wizard definition. showTitle in wizard step.

Renaming it would be a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. Well, that is a bummer. I was not aware it was used already. Nothing we can to at this point.

packages/pf4-component-mapper/src/files/wizard.js Outdated
@@ -157,7 +159,8 @@ WizardInternal.propTypes = {
closeButtonAriaLabel: PropTypes.string,
hasNoBodyPadding: PropTypes.bool,
navAriaLabel: PropTypes.string,
container: PropTypes.instanceOf(Element)
container: PropTypes.instanceOf(Element),
StepTemplate: PropTypes.func
Copy link
Member

Choose a reason for hiding this comment

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

What if somebody provides a class component? I would use the componentType

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't you think elementType?

Copy link
Member

Choose a reason for hiding this comment

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

yeah

@rvsia rvsia force-pushed the pf4StepTemplate branch from 88eaa4f to 3b62482 Compare July 14, 2020 10:54
@Hyperkid123 Hyperkid123 merged commit 2601568 into data-driven-forms:master Jul 14, 2020
@Hyperkid123
Copy link
Member

🎉 This PR is included in version 2.7.0 🎉

The release is available on

Demo can be found here!

@rvsia rvsia deleted the pf4StepTemplate branch August 5, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hyperkid123 Hyperkid123 Hyperkid123 approved these changes

Assignees

@Hyperkid123 Hyperkid123

Labels
enhancement New feature or request PF4 PF4 pull request released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rvsia @Hyperkid123