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

Update ActionForm to support enter key press #2935

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

skapur12
Copy link
Member

@skapur12 skapur12 commented Sep 30, 2024

This PR is to update the ActionForm so that the user can press the Enter key to submit a form rather than having to click the 'Sign In' button to log in.

This is to fix this CycleCloud bug: https://msazure.visualstudio.com/CycleCloud/_workitems/edit/29267683/

There is no corresponding CycleCloud PR for this change

Copy link

codecov bot commented Sep 30, 2024

Codecov Report

Attention: Patch coverage is 39.13043% with 14 lines in your changes missing coverage. Please review.

Project coverage is 80.27%. Comparing base (d43018f) to head (5d6fa93).

Files with missing lines Patch % Lines
...ages/bonito-ui/src/components/form/action-form.tsx 39.13% 14 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2935      +/-   ##
==========================================
- Coverage   80.41%   80.27%   -0.15%     
==========================================
  Files        1504     1504              
  Lines       48743    48751       +8     
  Branches     6074     6077       +3     
==========================================
- Hits        39197    39134      -63     
- Misses       9426     9433       +7     
- Partials      120      184      +64     
Flag Coverage Δ
80.27% <39.13%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ages/bonito-ui/src/components/form/action-form.tsx 68.96% <39.13%> (-5.04%) ⬇️

... and 27 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d43018f...5d6fa93. Read the comment docs.

if (onExecuteFailed) {
// Restore the form values on failure to avoid clearing, only if lastTypedValues exists
if (lastTypedValues) {
action.form.setValues(lastTypedValues);
Copy link
Member

Choose a reason for hiding this comment

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

Will the form value be cleared/ changed if the execution failed?

setSubmitting(true);
try {
// Store current form values before execution
setLastTypedValues(action.form.values);
Copy link
Member

Choose a reason for hiding this comment

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

lastTypedValues doesn't have to be a react state. Can be achieved by using a local variable

Suggested change
setLastTypedValues(action.form.values);
const lastTypedValues = action.form.values
...
action.form.setValues(lastTypedValues)


// Handle the Enter key submission
const handleKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
if (event.key === "Enter" && !submitting) {
Copy link
Member

Choose a reason for hiding this comment

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

Could consider adding an unit test

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.

2 participants