Skip to content

refactor: add opts and logic to allow submit on enter only under cuesheet line cell for time inputs#1945

Open
Shobhit-Nagpal wants to merge 2 commits intocpvalente:masterfrom
Shobhit-Nagpal:refactor/cuesheet/allow-submit-on-enter-only
Open

refactor: add opts and logic to allow submit on enter only under cuesheet line cell for time inputs#1945
Shobhit-Nagpal wants to merge 2 commits intocpvalente:masterfrom
Shobhit-Nagpal:refactor/cuesheet/allow-submit-on-enter-only

Conversation

@Shobhit-Nagpal
Copy link
Copy Markdown
Contributor

Changes made

  • added a allowSubmitOnEnterOnly field under opts for useReactiveTextInput hook
  • passing an optional parameter of triggeredKey under handleSubmit to keep track of whether user pressed submit or not
  • added an else-if check under handleSubmit which implements the behavior to cancel update unless user pressed enter or mod+enter to submit the same value
  • passed the allowSubmitOnEnterOnly option as true under <SingleLineCell /> under cuesheet-table-elements

Working demo:

ontime-cuesheet-submit-on-enter-only-1-2026-01-15_22.16.31.mp4

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 15, 2026

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

triggeredKey !== 'Enter' &&
triggeredKey !== 'mod + Enter'
) {
options?.onCancelUpdate?.();
Copy link
Copy Markdown
Collaborator

@alex-Arc alex-Arc Jan 20, 2026

Choose a reason for hiding this comment

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

It seams that we don't provide a function here
https://github.com/Shobhit-Nagpal/ontime/blob/de5e2e79f9100eaa47f43724ee681b8495e9e21b/apps/client/src/views/cuesheet/cuesheet-table/cuesheet-table-elements/cuesheetColsFactory.tsx#L189

witch result is potentially stale data
if you make a change and then press tab the edit will remain but it is not submitted

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i think this was being prevented with the check for initial value and the new value before. if there's a change in the value, it would not enter this statement block

even if we were to pass in the cancel callback, it should submit the value in case there's a change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i can think of 2 ways to handle this.

  • can bring the equality check of new value and initial value and add in a comment to mention why the check is present
  • can add in another flag called checkForSameValue and based on if this is true, then check if the value is same or not and proceed further (but this feels like a long way to do the same thing as above)

do you have any other suggestions on this? @alex-Arc

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

first another question, what do we mean by options?.allowSubmitOnEnterOnly ?
what I read is that only the enter key is allowed to submit this field, that is also what the following checks suggest
options?.allowSubmitOnEnterOnly && triggeredKey !== 'Enter' && triggeredKey !== 'mod + Enter'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

allowSubmitOnEnterOnly means to only allow submission of the value if the user has pressed enter or mod + enter.

Suppose this flag is false, we're accepting submission on Escape and Tab as well. The triggeredKey is to be able to know which key was pressed when navigating the cuesheet to so we can confirm if the key is an Enter or mod + Enter key

The scenario would be that the user is active on a time input and they tab to the next input without changing any value, it would still submit. We want to allow a submission only on the enter key and not allow tab or escape to enter

So we pass in the flag of allowSubmitOnEnterOnly as true and check which key was triggered while navigating to then confirm if we want to submit the value or not

Do let me know if I can be more clear on a specific part

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good I understood correctly,
In that case the logic as it is now is correct, we just need to supply the onCancelUpdate so that it actually works

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, but the edge case you mentioned where if the user has changed the value and then they tab, the value won't be submitted is not what we want. in any case that the value changes, we want it to submit

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Then allowSubmitOnEnterOnly is not a good name

looking further back into it, is the issue not that we have some formatting in the time filed that automatically get a value when you tab into them?
should the fix perhaps be isolated to those inputs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the issue is that when the value doesn't change on time inputs and the user tabs / escapes on the active time field, there's a submission of the value that happens

i did try scoping the issue to the time field inputs but the issue is that there's a prop of lockedValue which re-renders the component and it keeps changing. if not that, the input has an isEditing check on which component to display (input or div). this gets the component to lose its memory of what to do

basically, what we need to do is:

  • check if the value is same or not
  • if the user pressed escape / tab AND the value is same, then don't submit
  • if the value is different, submit in either case

the reason this logic above isn't satisfied by the allowSubmitSameValue prop is because it's linked to the value being locked or not which keeps changing when we're active on the time inputs. to not change the current behavior and also be able to add this fix, added a flag to check for this condition. from what i've tried, scoping it to the input field itself might make the code more complicated and i'm not sure on where to start with that either

happy to take in suggestions or improvements on the name or the overall logic

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