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.

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

Tests and docs for amplify-text-field #3671

Merged
merged 11 commits into from
Jul 17, 2019
Merged

Tests and docs for amplify-text-field #3671

merged 11 commits into from
Jul 17, 2019

Conversation

tjleing
Copy link
Contributor

@tjleing tjleing commented Jul 16, 2019 •
edited
Loading

This PR adds Stencil and Storybook integration (with Stencil end-to-end tests) to the amplify-text-field component.

Storybook component

Unfortunately all the yarn version-ing created a bunch of useless commits.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

tjleing
tjleing commented Jul 16, 2019
View reviewed changes
packages/amplify-ui-components/src/components/amplify-text-field/amplify-text-field.tsx
type="text"
{...this.inputProps}
type={this.inputProps.hasOwnProperty('type') ? this.inputProps.type : 'text'}
inputProps={{ ...this.inputProps }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another weird one. Originally, the code was {...this.inputProps}, which isn't correct because the <amplify-text-input> component is also looking to receive inputProps={type?: str, onInput?: Function}. I figured there could conceivably be some reason why just saying inputProps={this.inputProps} would mess up the component, but does anyone know if it actually will?

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried it ? :)

@tjleing
Copy link
Contributor Author

tjleing commented Jul 16, 2019

One more comment, I have no clue why the CircleCI build is failing... Is this the problem people were talking about earlier, or what? People online were saying that running jest with -w=2 fixed the problem, but I have no clue how or why

@tjleing
Copy link
Contributor Author

tjleing commented Jul 17, 2019

Yet another comment: we should decide what tests to put in --spec and what tests to put in --e2e, I kinda just lumped them all into e2e when some of them should probably be --spec.

@sammartinez sammartinez added the UI Related to UI Components label Jul 17, 2019
@tjleing tjleing requested a review from sammartinez July 17, 2019 18:13
sammartinez
sammartinez reviewed Jul 17, 2019
View reviewed changes
package.json Show resolved Hide resolved
packages/amazon-cognito-identity-js/package.json Show resolved Hide resolved
packages/amplify-ui-components/package.json Show resolved Hide resolved
packages/amplify-ui-components/package.json Show resolved Hide resolved
packages/amplify-ui-components/src/components/amplify-text-field/amplify-text-field.e2e.ts Show resolved Hide resolved
packages/amplify-ui-components/src/components/amplify-text-field/amplify-text-field.e2e.ts Show resolved Hide resolved
packages/amplify-ui-components/src/components/amplify-text-field/amplify-text-field.tsx
type="text"
{...this.inputProps}
type={this.inputProps.hasOwnProperty('type') ? this.inputProps.type : 'text'}
inputProps={{ ...this.inputProps }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried it ? :)

@jordanranz
Copy link
Contributor

jordanranz commented Jul 17, 2019 •
edited
Loading

One more comment, I have no clue why the CircleCI build is failing... Is this the problem people were talking about earlier, or what? People online were saying that running jest with -w=2 fixed the problem, but I have no clue how or why

I believe there was a change to Stencil that reserves the word 'change'. Can you change the word 'change' in rock-paper-scissors.tsx to 'iconChange'

@tjleing tjleing merged commit c8da1e4 into aws-amplify:amplify-ui-components Jul 17, 2019
tjleing pushed a commit that referenced this pull request Jul 23, 2019
Initial commit to fix #3671
a8586df
sammartinez added a commit that referenced this pull request Aug 6, 2019
@sammartinez
Ui components/form (#3790)
089891e 
* initial commit for amplify ui web components

* Add base styling and cx to emotion css files

* v0.1.31

* v0.1.30

* v0.2.0

* v0.1.30

* v0.1.99

* Tests added to amplify-text-field; prettifier run

* v0.1.30

* Typo fix

* (partly just to fix CircleCI): Implement solution #4 on https://github.com/aws-amplify/amplify-js/pull/3671/files#r304138732

* Cleaner code

* change () -> iconChange () for CircleCI, hopefully it works

* Add styling to web components

* UI component build fix (#3691)

* Amplify stencil components

* New components

* Generate component docs

* Update es module to match script

* Trying amplify-scene with Amplify imported

* Update stencil to 1.0

* feat: amplify-scene and readme build

* feat: add storybook and button story

* feat: storybook integration and button component changes

* Fix Vue build error and update yarn lock

* Remove exports

* Remove package lock from core

* Initial commit to fix #3671

* Move amplify-ui-components package from npm to yarn (#3718)

* Clean up linting (#3711)

* Remove husky pre-hooks until linting is functioning

* Clean up linter

* Revert quotation change

* Add screenshot testing (#3736)

* Move amplify-ui-components package from npm to yarn

* Add visual screenshot testing to amplify-text-field, and fix jest-config problem manually

* Bump stencil version + husky pre-push testing

* Bump resource class for CircleCI

* fix typo in constants

* Update LICENSE
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jordanranz jordanranz jordanranz left review comments

@sammartinez sammartinez sammartinez left review comments

Assignees

@tjleing tjleing

Labels
UI Related to UI Components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tjleing @jordanranz @sammartinez