-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix: Make all task titles/descriptions read-only to all but the author/assignee. #53749
fix: Make all task titles/descriptions read-only to all but the author/assignee. #53749
Conversation
…r/assignee. Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
@Expensify/design @thienlnam, before recording the videos, I want to confirm if the disabled cursor looks correct or not. I feel it might confuse users, making them think they can't click the link (which we want them to click). Context: When users accidentally edit the description of a task assigned to them, it unexpected and feels broken, undermining the perception of product quality and conversion. So we decided to disable editing the task fields if the task created by Concierge, users will be only able to complete/incomplete the tasks. Monosnap.screencast.2024-12-09.16-18-14.mp4 |
@shawnborton @dubielzyk-expensify @dannymcclain, could you please check the comment above🙏🏻? I think |
We don't need to use the Check out this screenshot from an expense. I can no longer change the merchant or date, but I can still change the description, category, and tag. My cursor is totally normal when I hover over the merchant and date rows, they just don't do anything. |
Just realized a video would probably be more clear: CleanShot.2024-12-10.at.08.25.27.mp4 |
Totally agree with that. Your video is exactly what we want to happen with tasks. |
Agree with the designers 👍 |
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
@Expensify/design, does this look correct now? Monosnap.screencast.2024-12-11.09-49-29.mp4 |
Looks right to me 👍 |
Yeah that looks right to me too 👍 |
Yup, looks good to me three! |
Thanks for the confirmation, I will add the recordings today. |
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
@Expensify/design, please review the behaviour new behaviour below and let me know if I'm still missing something. Thanks! Monosnap.screencast.2024-12-17.05-17-42.mp4 |
That looks right to me. Gut check from @Expensify/design |
Yup, looks good to me too! |
Just so I'm totally understanding: have we updated the tasks so that only the author or the person assigned can complete/uncomplete the task? Just wanted to double-check that behavior, other than that, it looks good to me too. |
I don't know if we updated anything there? My understanding is that we didn't touch that part of the task UX. Basically if you have access to view a task (aka if the message/thread was shared with you), you have access to complete or uncomplete it. I think that's how it's always been though? |
Ah, great catch on that - @Krishna2323 what are your thoughts there? |
Correct, we haven't changed that implementation.
Nope, task can be only marked as complete/incomplete only by the author and assignee. |
Okay cool, as long as we aren't changing anything about that behavior then we're all good 👍 |
@suneox, you can start reviewing again, I will update the test steps and recording today. |
Sure I'll review pr again within a few hours |
@Krishna2323 The changes look good; please proceed to update ESLint. |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2024-12-22.at.23.25.35.mp4MacOS: DesktopScreen.Recording.2024-12-22.at.23.28.36.mp4Android: NativeScreen.Recording.2024-12-22.at.23.37.50.mp4Android: mWeb ChromeScreen.Recording.2024-12-22.at.23.36.31.mp4iOS: NativeScreen.Recording.2024-12-22.at.23.32.59.mp4iOS: mWeb SafariScreen.Recording.2024-12-22.at.23.34.16.mp4 |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
2 similar comments
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
1 similar comment
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.78-0 🚀
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.78-6 🚀
|
Explanation of Change
Fixed Issues
$ #52979
PROPOSAL: #52979 (comment)
Tests
Concierge
chat.Offline tests
Concierge
chat.QA Steps
Concierge
chat.PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
android_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop
desktop_app.mp4