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

Refactor: use form onSubmit for ConfirmorCancel #1617

Closed
vaibhavgarg237 opened this issue Oct 14, 2022 · 4 comments
Closed

Refactor: use form onSubmit for ConfirmorCancel #1617

vaibhavgarg237 opened this issue Oct 14, 2022 · 4 comments
Assignees
@vaibhavgarg237
Labels
enhancement New feature or request
Milestone
v1.21

Comments

@vaibhavgarg237
Copy link
Contributor

vaibhavgarg237 commented Oct 14, 2022 •
edited by escapedcat
Loading

Description

Currently for calling Confirm from ConfirmorCancel component we use onClick as a prop, due to which user can't use Enter key to submit the form, wrapping it inside a form would be better.

Describe the solution

Wrapping all the ConfirmorCancel components forms with forms and handling submit operation via onSubmit instead of onClick

Describe alternatives

After merging PR #1576, onClick will be made optional after which we could wrap all occurences inside forms and handle onSubmit operation accordingly.

Additional context [optional]

Refer PR #1576
@vaibhavgarg237 vaibhavgarg237 added the enhancement New feature or request label Oct 14, 2022
@vaibhavgarg237
Copy link
Contributor Author

Will open a PR for this after PR #1576 gets merged to avoid optional onClick related merge conflict

@escapedcat
Copy link
Contributor

Hey @vaibhavgarg237 you still wanna do this one?

@vaibhavgarg237
Copy link
Contributor Author

vaibhavgarg237 commented Dec 4, 2022 •
edited
Loading

Yeah, sorry I forgot about this issue. I have updated the PR.

vaibhavgarg237 added a commit to vaibhavgarg237/lightning-browser-extension that referenced this issue Dec 4, 2022
@vaibhavgarg237
refactor: form el for ConfirmorCancel getAlby#1617
415c94d
@vaibhavgarg237 vaibhavgarg237 mentioned this issue Dec 4, 2022
Merged
3 tasks
@escapedcat escapedcat added this to the v1.21 milestone Dec 5, 2022
vaibhavgarg237 added a commit to vaibhavgarg237/lightning-browser-extension that referenced this issue Dec 5, 2022
@vaibhavgarg237
refactor: form el for ConfirmorCancel getAlby#1617
e53c1c5
escapedcat added a commit that referenced this issue Dec 8, 2022
@escapedcat
Merge pull request #1816 from vaibhavgarg237/submitConfirm
72c55d0 
refactor: form el for ConfirmorCancel #1617
@bumi
Copy link
Collaborator

bumi commented Dec 11, 2022

PR merged

@bumi bumi closed this as completed Dec 11, 2022
bumi added a commit that referenced this issue Dec 12, 2022
@bumi
Merge branch 'master' into feature/request-method
99fbf0b 
# By Jan Kögel (6) and others
# Via GitHub (9) and Michael Bumann (1)
* master:
  Update all development Yarn dependencies (2022-12-10)
  refactor: ABC again!?
  refactor: ABC.
  refactor: toLowerCase should be what we want here.
  feat: load all dayjs I18n files.
  added Lightsats and LNflip, removed Sparkshot
  Translated using Weblate (Portuguese (Brazil))
  chore: lowecase as it should be
  v1.20.1
  prettier
  fix: local config
  refactor: getOS: don't use deprecated appVersion.
  fix: make sure we use correct locales to format the numbers
  Update react-router-dom to version 6.4.4
  chore: add 25k option for the default amounts
  refactor: form el for ConfirmorCancel #1617

# Conflicts:
#	src/app/screens/ConfirmRequestPermission/index.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vaibhavgarg237 vaibhavgarg237

Labels
enhancement New feature or request
Projects
None yet
Milestone
v1.21
Development

No branches or pull requests
3 participants
@bumi @escapedcat @vaibhavgarg237