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

Feature/repeat pages #34

Merged
merged 6 commits into from Jul 20, 2020
Merged

Feature/repeat pages #34

merged 6 commits into from Jul 20, 2020

Conversation

jenbutongit
Copy link
Contributor

@jenbutongit jenbutongit commented Jul 1, 2020

Description

Repeatable pages support! This is currently a developer only feature (so no support in designer for it right now.)

  • Repeatable pages can be flagged with "repeatableField" property which the field storing 'n'.
  • These values are stored as an array of objects inside section. The objects key the values by page path sectionName: [{/page-1: {...}, /page-2: {...}}] to keep track of which pages have been "completed". Each array item is 1 "iteration" of the repeatable section.
  • Support for summary
    • summary-detail has been split out into summary-row
    • Added isArray filter to detect if the section...is an array (repeated)
  • waiting on a patch from @superafroman also
  • change circleci to trigger engine build + test

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Use dynamic.json inside runner/test/cases if you would like to "play" a repeatable form.

  • Unit tested

Checklist:

  • My code follows the javascript standard style
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and versioning
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have rebased onto master and there are no code conflicts

@jenbutongit jenbutongit force-pushed the feature/repeat-pages branch 2 times, most recently from bb2222a to 2034034 Compare July 1, 2020 10:50
@jenbutongit jenbutongit added the do not merge Not ready to merge! label Jul 1, 2020
@jenbutongit jenbutongit force-pushed the feature/repeat-pages branch 2 times, most recently from a3ee685 to 2830ea9 Compare July 2, 2020 10:34
@jenbutongit jenbutongit removed the do not merge Not ready to merge! label Jul 2, 2020
const updateValue = { [this.path]: update[this.section.name] }
const sectionState = state[this.section.name]
let updateValue = {[this.path]: update[this.section.name]}
let sectionState = state[this.section.name]
Copy link
Member

Choose a reason for hiding this comment

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

Is this an auto format thing? Should be const from what I can see?

if (!sectionState) {
update = { [this.section.name]: [updateValue] }
} else if (!sectionState[num - 1]) {
} else if(!sectionState[num-1]) {
Copy link
Member

Choose a reason for hiding this comment

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

Auto formatting again? Spacing heere is good - don't we have a standard formatter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe merge issue, sorry! Might be that engine doesn't have linter installed and/or not running on test. I'll add.

const nextLink = this.next.find(link => {
const { page, condition } = link
const value = page.section ? state[page.section.name] : state
getNextPage (state, suppressRepetition = false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

neat, didn't know we could do parameter defaulting :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JS is a lot nicer these days :)

@jenbutongit
Copy link
Contributor Author

It looks like babel-eslint/eslint wont be supporting private methods until babel v8. babel/babel#10752, babel/babel-eslint#801. Ignoring for now.

jenbutongit and others added 5 commits July 10, 2020 10:23
…quashed commits)

Squashed commits:
[46905cf] Fix duplicate questions when formatting webhook data.
[cbb0fb4] Fix webhook data type.
[688ccfe] Ignore EPIPE error when uploading documents.

This error is returned when a response has been received before the request has finished sending.
[2034034] lint
[61ad037] oops. get rid of these
[9aab773] move Key into object so it can be stubbed nicely. dynamic tests
[2f81e9c] don't push all repeatable data to summary
[b139a85] Webhook data updated with repeatable logic
[9973cbb] Summary page working with repeatable pages. Section headers correct.
[4de54bb] fix webhook data
[a2badf6] oops. reset counter was in wrong place
[8a744ff] oops revert to this
[f26c95d] split out row
[5b55fa7] revert test.json, dynamic pages looping + reading values. return
[fceaae9] wip.. almost..!
[0298514] summary does not error, dont rely on null value for summary.js..!
[4349bfa] wip
[af7bf4d] private method/props. reading state for repeatable pages
[40d7174] state wip
[f79ab6b] looping, probably
[dd31960] wip
[dba8b3a] wip
[a007995] sourcemaps, add hoek
[993f834] **Runner**
- N/A

**Designer**
- N/A

**Engine**
- babel for engine. export everything in index
@jenbutongit jenbutongit merged commit 6412614 into master Jul 20, 2020
@jenbutongit jenbutongit deleted the feature/repeat-pages branch July 20, 2020 10:02
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.

None yet

3 participants