Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Simplified Collect][Taxes] Tax editing and bulk actions #38208
[Simplified Collect][Taxes] Tax editing and bulk actions #38208
Changes from 30 commits
dff681d
ff731cb
ea97afb
f512cf0
2a3f4c3
f40c5b5
a25926f
ac59d75
76d56f8
75c48ba
3b590c6
ed94efe
b8eb432
27ecd7e
2cee1ee
75f7f03
f7a6213
27ddedf
f8992bd
561c651
472d983
3f55637
ddee558
1bac4db
2fc8fee
110f743
89f474c
d0164a4
70f726e
94a3111
b9dce2f
5876e11
52688aa
8cadf35
089661e
e6335cd
ef278f1
0c68d88
acd8d58
d97bcd3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAB the url changes when navigating from Edit to Name/Value pages. We can address this as a follow up though
Screen.Recording.2024-03-19.at.10.04.53.AM.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing es translations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asked for translations here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Expensify/marketing could you please review this copy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To confirm, these are just generic error messages where we don't really know exactly what happened? Or is there any more info we have where we can get more specific? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, these are just generic for when we don't know what happened. Otherwise the server sends specific error messages for the cases we do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if needed we can be a bit more specific for
updateFailureMessage
. We can split it:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would guess, though, that we'd show the error message right after you take the action that triggered it, so it would be fairly easy to understand what went wrong. Maybe we can stick with the generics here:
deleteFailureMessage
: An error occurred while deleting the tax rate. Please try again or ask Concierge for help.updateFailureMessage
: An error occurred while updating the tax rate. Please try again or ask Concierge for help.createFailureMessage
: An error occurred while creating the tax rate. Please try again or ask Concierge for help.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, agreed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
valuePercentageRange
isn't confirmed in the Slack.