Revert "Allow edit input for SplitListItem when there is negative value"#81717
Revert "Allow edit input for SplitListItem when there is negative value"#81717
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
Sorry for the noice @nkuoch @heyjennahay 🙏 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bfd5582b2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function calculateSplitAmountFromPercentage(totalInCents: number, percentage: number): number { | ||
| // Calculate directly without clamping to support negative percentages | ||
| // This naturally handles all sign combinations: | ||
| // - Positive total × Positive % = Positive split (normal expense share) | ||
| // - Positive total × Negative % = Negative split (person owes money back) | ||
| // - Negative total × Positive % = Negative split (person gets refund share) | ||
| // - Negative total × Negative % = Positive split (person owes money on a refund) | ||
| const amount = Math.round((totalInCents * percentage) / 100); | ||
| const totalAbs = Math.abs(totalInCents); | ||
| // Clamp percentage to [0, 100] without rounding to preserve decimal precision | ||
| const clamped = Math.min(100, Math.max(0, percentage)); | ||
| const amount = Math.round((totalAbs * clamped) / 100); |
There was a problem hiding this comment.
Do not clamp split percentages while edits allow >100
The split-by-percentage input still enables allowExceedingHundred (see SplitPercentageInput), but calculateSplitAmountFromPercentage now clamps percentage to [0,100]. When a user types 110–150% as a temporary edit (the existing UX allows this during multi-split adjustments), the amount is computed as 100%, so the amount no longer reflects the user’s input and the value snaps back when the draft clears. This makes it impossible to use the temporary >100% edits that the UI still accepts.
Useful? React with 👍 / 👎.
| prefixCharacter={symbol} | ||
| prefixStyle={styles.colorMuted} | ||
| keyboardType={props.keyboardType ?? CONST.KEYBOARD_TYPE.DECIMAL_PAD} | ||
| keyboardType={CONST.KEYBOARD_TYPE.DECIMAL_PAD} | ||
| // On android autoCapitalize="words" is necessary when keyboardType="decimal-pad" or inputMode="decimal" to prevent input lag. |
There was a problem hiding this comment.
Keep keyboardType override for negative split edits
Hardcoding keyboardType to DECIMAL_PAD removes the ability for callers (e.g., split amount inputs) to request a keyboard with a minus key. On iOS, decimal-pad doesn’t include “-”, so if a split amount is negative (refunds/adjustments) and the user clears or replaces the text, they can’t re-enter the negative sign. Previously SplitAmountInput passed a numbers-and-punctuation keyboard to handle this case.
Useful? React with 👍 / 👎.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Reverts #74189