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

feat: add mutations #371

Merged
merged 4 commits into from
Jan 31, 2022
Merged

Conversation

dpilch
Copy link
Member

@dpilch dpilch commented Jan 28, 2022

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dpilch dpilch force-pushed the workflow/mutation branch 2 times, most recently from f251c90 to 0e1becb Compare January 31, 2022 18:05
@dpilch dpilch force-pushed the workflow/mutation branch from 0e1becb to 060edea Compare January 31, 2022 20:51
@dpilch dpilch changed the title [WIP] feat: add mutations feat: add mutations Jan 31, 2022
Copy link
Contributor

@alharris-at alharris-at left a comment

Choose a reason for hiding this comment

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

Overall LGTM - just a few minor questions/comments outside of the existing TODOs.

packages/codegen-ui/lib/types/studio-types.ts Show resolved Hide resolved
.map(([key]) =>
buildOpeningElementEvents(
{ bindingEvent: getSetStateName({ componentName: this.component.name || '', property: key }) },
'change', // TODO: use component event mapping
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a usecase where this isn't going to be change? I'm thinking TextField, CheckBoxField, etc. are all basically just listening to the value 'change' event. It doesn't make sense for this to be applied to anything that isn't a field, right?

@alharris-at alharris-at marked this pull request as ready for review January 31, 2022 22:13
@alharris-at alharris-at requested a review from a team January 31, 2022 22:13
@codecov-commenter
Copy link

Codecov Report

Merging #371 (686511e) into tagged-release/q1-release (f35ebca) will decrease coverage by 0.25%.
The diff coverage is 85.50%.

Impacted file tree graph

@@                      Coverage Diff                      @@
##           tagged-release/q1-release     #371      +/-   ##
=============================================================
- Coverage                      93.35%   93.10%   -0.26%     
=============================================================
  Files                             39       40       +1     
  Lines                           1460     1566     +106     
  Branches                         317      339      +22     
=============================================================
+ Hits                            1363     1458      +95     
- Misses                            95      106      +11     
  Partials                           2        2              
Impacted Files Coverage Δ
packages/codegen-ui/lib/renderer-helper.ts 100.00% <ø> (ø)
packages/codegen-ui/lib/types/studio-types.ts 100.00% <ø> (ø)
...s/codegen-ui-react/lib/react-component-renderer.ts 78.26% <66.66%> (-8.23%) ⬇️
...egen-ui-react/lib/react-component-render-helper.ts 88.50% <67.74%> (-2.52%) ⬇️
packages/codegen-ui-react/lib/workflow/mutation.ts 90.90% <90.90%> (ø)
...react/lib/amplify-ui-renderers/amplify-renderer.ts 100.00% <100.00%> (ø)
...eact/lib/react-component-with-children-renderer.ts 95.00% <100.00%> (+0.88%) ⬆️
...gen-ui-react/lib/react-studio-template-renderer.ts 91.27% <100.00%> (+0.12%) ⬆️
packages/codegen-ui-react/lib/workflow/action.ts 94.91% <100.00%> (+13.96%) ⬆️
packages/codegen-ui-react/lib/workflow/index.ts 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 f35ebca...686511e. Read the comment docs.

@alharris-at alharris-at merged commit c62df4d into tagged-release/q1-release Jan 31, 2022
@alharris-at alharris-at deleted the workflow/mutation branch January 31, 2022 23:29
@dpilch dpilch mentioned this pull request Feb 24, 2022
@alharris-at alharris-at mentioned this pull request Feb 24, 2022
alharris-at added a commit that referenced this pull request Feb 24, 2022
* feat: add mutation types

* feat: mutations

* fix: add properties check to address failing integ test

* chore: update types and remove replacing flat call with flatMap

Co-authored-by: Alexander Harris <alharris@amazon.com>
alharris-at added a commit that referenced this pull request Feb 25, 2022
* feat: add mutation types

* feat: mutations

* fix: add properties check to address failing integ test

* chore: update types and remove replacing flat call with flatMap

Co-authored-by: Alexander Harris <alharris@amazon.com>
alharris-at added a commit that referenced this pull request Feb 25, 2022
* feat: add mutation types

* feat: mutations

* fix: add properties check to address failing integ test

* chore: update types and remove replacing flat call with flatMap

Co-authored-by: Alexander Harris <alharris@amazon.com>
alharris-at added a commit that referenced this pull request Feb 25, 2022
* feat: add mutation types

* feat: mutations

* fix: add properties check to address failing integ test

* chore: update types and remove replacing flat call with flatMap

Co-authored-by: Alexander Harris <alharris@amazon.com>
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.

3 participants