-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Report Fields] Update 'deletable' fields to show as "remove field " vs. "empty" #35331
Comments
@caitlinwhite1 Can I please get a UI screen of how this |
@allroundexperts Yup! Let me grab something from Figma |
@allroundexperts I updated with the context from that slack thread but let me know if you need any other info! |
Thanks @caitlinwhite1. I was waiting for my other PR related to backend integration to get merged first. It got in Yesterday so I should be making a PR for this soon! |
Update: Created a draft PR here. Waiting on @thienlnam to provide me with the API endpoint needed to delete the report field. |
Hi @thienlnam! |
Yup, it's merged pending deploy - should be available on staging tomorrow. Here are those details Endpoint: RemoveReportField
|
Not sure, but it's on production now so you should be able to test there. Also as a note - due to the way these are stored, you'll have to prefix the fieldID with expensify_ So same as the changes I made here #36769 |
@allroundexperts quick note that we are trying to wrap up the report fields initiative by the end of this month. Do you think it'll be possible to get this PR merged by then? |
@JmillsExpensify This is blocked on #36170 which is waiting changes from the backend. I'm not sure how long it will take for those changes to get in. This can not be completed until those are done. |
Nice, appreciate the color. I'm also going to go ahead and move this to our |
Circling back on this one, with the PR for #36170 now merged, are we good to pick this one back up? |
Yes. I'll try to resolve the conflicts today and make it ready for review. |
Hit production 2024-04-10, payment should be due tomorrow. cc @JmillsExpensify |
This issue has not been updated in over 15 days. @allroundexperts, @thienlnam eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
This deployed so i think we're all set to close! |
@caitlinwhite1 Please can you reopen as payment is due for the PR review? Thanks 🙏 |
Triggered auto assignment to @dylanexpensify ( |
@dylanexpensify reopening and adding bug label for payrment! |
Nice one! Payment summary:
Please apply or request! |
Thanks, applied @dylanexpensify! |
@dylanexpensify Can you write up my summary as well? 😄 |
Yes! |
@jjcoffee sent offer! Also @allroundexperts updated original post btw! |
@dylanexpensify Accepted, cheers! |
Paid @jjcoffee! |
$500 approved for @allroundexperts |
The current funtionailty for Report Fields allows the user to set a value as "empty" to delete the field. We'll want to update the UI so it's more apparent to the end user. More contect in this thread,
If a report field exists within a report but has been removed from the policy, mark the report field as 'deletable' by showing "Remove field" in the overflow menu for each individual report field that becomes deletable. For Example:
The text was updated successfully, but these errors were encountered: