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

Button/TxButton submit on enter #1170

Merged
merged 12 commits into from
May 22, 2019
Merged

Button/TxButton submit on enter #1170

merged 12 commits into from
May 22, 2019

Conversation

kwingram25
Copy link
Contributor

Closes #1162

Add submitOnEnter prop to trigger enter-submit for Button and TxButton and activate on most forms

Needs testing to ensure no event conflicts or side effects, seems safe so far

jacogr
jacogr previously requested changes May 15, 2019
Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

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

As discussed, would prefer a solution (not sure what that looks like), where -

  • there are no global listeners being added
  • we can still use keys on Inputs (there are fine)
  • we can trigger the button to fire

It does have downsides, i.e. enter on a modal with no input, will never submit. But that is also fine. Element of least surprise.

@kwingram25
Copy link
Contributor Author

kwingram25 commented May 16, 2019

Testing an approach using refs to trigger buttons/txButtons from Input.onEnter() - currently on Transfer.tsx and Vanity.tsx only

This feels less elegant in my opinion with a lot of nearly identical code bits to add - also needs a lot more work on each possible input component, for example text inputs on InputExtrinsic. Would prefer to keep document listeners and use window vars to prevent accidentally stacking listeners

@jacogr jacogr dismissed their stale review May 16, 2019 13:48

stale.

@kwingram25
Copy link
Contributor Author

Add TxComponent to link Button/TxButton ref (this.button) with enter events on text inputs (this.submit / this.sendTx)

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

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

lgtm, only had a brief play, but it looks sane.

@jacogr
Copy link
Member

jacogr commented May 20, 2019

Some conflicts snuck in.

@jacogr
Copy link
Member

jacogr commented May 21, 2019

@Tbaut Since you logged the issue, would like you to play with it as well. (Hence have not merged as-is - it feels fine to me)

Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

I got an Uncaught TypeError: Cannot read property 'click' of null a page crash when clicking on the "change password" button in account>edit.

Anything else looks good.

@kwingram25
Copy link
Contributor Author

Had to move the ref from class method to constructor, strange bug where it was null on modals

@Tbaut Tbaut self-requested a review May 22, 2019 15:41
Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Works well :)

  • tested the password change
  • transfer
  • backup download

@kwingram25 kwingram25 merged commit dc943d7 into master May 22, 2019
@jacogr jacogr deleted the ki-form-enter branch May 22, 2019 21:57
jacogr added a commit that referenced this pull request May 23, 2019
* Button/TxButton submit on enter (#1170)

* Button/TxButton submit on enter

* promise linting

* testing ref approach

* Add TxComponent to handle submit on enter

* remove accountNonce

* remove input ref

* remove TxButton constructor

* remove comment

* empty constructor

* linting

* bug with null ref

* linting

* [CI Skip]  0.33.0-beta.74

* Redirect correctly when user has no account (#1191)

* right link and hide tabs plus buttons primary

* fix comments

* create highlight

* [CI Skip]  0.33.0-beta.75

* Mark ExtrinsicFailed as error (#1190)

* Hide edit on injected accounts, Button w/ tooltip (#1192)

* Hide edit on injected accounts, Button w/ tooltip

* Don't display "add tags" when not isEditable

* Indentation

* [CI Skip]  0.33.0-beta.76

* Make enable() call to extensions (#1194)

* Make enable() call to extensions

* It works

* Add overlay for injected (prompt)

* Use signer from injectedPromise

* [CI Skip]  0.33.0-beta.77

* withInjected -> PureComponent (fix new refs) (#1195)

* withInjected -> PureComponent (fix new refs)

* Consume data from context

* Expand injected

* Close modal on tx start
Tbaut added a commit that referenced this pull request May 24, 2019
* starting point

* show all accounts

* edit and save a name but remains ugly

* actually don't use deriveStateFromProps

* fix AdressRow

* indentation

* upperCase

* bring back the index

* trim input

* change edition l&f

* lint

* cater for small screens

* address comments

* lint and alphabetical order

* allow viewing and editing tags

* better UX when tag editing and styling for no tags

* nonce refactoring and show balances

* crypto type

* styling balances

* use css grid

* fix address row and creator

* with buttons

* button styling

* locked tooltip

* edit button always visible

* init

* fix conflicts

* transfer functionnality

* styling the modal

* clean up

* clean up bis

* Transfer dialog enhancements (#1196)

* Button/TxButton submit on enter (#1170)

* Button/TxButton submit on enter

* promise linting

* testing ref approach

* Add TxComponent to handle submit on enter

* remove accountNonce

* remove input ref

* remove TxButton constructor

* remove comment

* empty constructor

* linting

* bug with null ref

* linting

* [CI Skip]  0.33.0-beta.74

* Redirect correctly when user has no account (#1191)

* right link and hide tabs plus buttons primary

* fix comments

* create highlight

* [CI Skip]  0.33.0-beta.75

* Mark ExtrinsicFailed as error (#1190)

* Hide edit on injected accounts, Button w/ tooltip (#1192)

* Hide edit on injected accounts, Button w/ tooltip

* Don't display "add tags" when not isEditable

* Indentation

* [CI Skip]  0.33.0-beta.76

* Make enable() call to extensions (#1194)

* Make enable() call to extensions

* It works

* Add overlay for injected (prompt)

* Use signer from injectedPromise

* [CI Skip]  0.33.0-beta.77

* withInjected -> PureComponent (fix new refs) (#1195)

* withInjected -> PureComponent (fix new refs)

* Consume data from context

* Expand injected

* Close modal on tx start

* align fees

* remove blank lines
@polkadot-js-bot
Copy link

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

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enter key submits the form
5 participants