-
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
Move Pin Action Into Context Menu and Remove Pin Button #22635
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01229c410d2206c52f |
Triggered auto assignment to Contributor Plus for review of internal employee PR - @sobitneupane ( |
I looked into this, and it's going to require some refactoring. I think we should handle this similarly to the report action context menus where we display different options depending on the reportAction (except here, we would display different options depending on the report) |
@JmillsExpensify could you please help me locate the desired pin button / three dots menu item behavior for all report types? |
Apologies, just seeing this one. |
This is a bit tricky, since some of these haven't been mocked up. I'll go ahead and assign myself and come back to this. |
Should we hold this? @JmillsExpensify |
@shawnborton @dannymcclain Can one of you take this on by chance? I haven't been able to get to it. |
Sure, let's spin the wheel! |
Triggered auto assignment to @shawnborton ( |
Maybe we could combine the call options into the same menu with pin? cc @trjExpensify as well |
Yeah, I think that works instead of an additional level. Once we have in-app calling & screenshare built, I think we remove the Zoom and Google Meet options completely. |
Nice, I agree on this path, including deprecating Meet and Zoom once we support in-app calling. |
@JmillsExpensify can I get a priority gut check for this? |
With the design hashed out, can we make it |
@trjExpensify good shout, I don't see why not. Can we confirm that the above screenshots are the final design with options:
And then we need to know:
|
Actually I think we have some overlap here, pretty sure @srikarparsi submitted a PR for this already in relation to some thread/subscribe/unsubscribe changes? |
Oh yup right here: #27748 |
Dope, let's close this then or link that PR to this issue perhaps? |
Going to close this since this one is live already. Nice work @srikarparsi |
Part of #20486 (comment)
Relevant:
#22332 (comment)
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: