Skip to content
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

[HOLD for production 2024-10-07][$250] [Search v2.3] Add the bookmark icon for Saved searches on mobile #49442

Closed
lakchote opened this issue Sep 19, 2024 · 22 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lakchote
Copy link
Contributor

lakchote commented Sep 19, 2024

See #48566 (comment)

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021836682788967357264
  • Upwork Job ID: 1836682788967357264
  • Last Price Increase: 2024-09-19
Issue OwnerCurrent Issue Owner: @VictoriaExpensify
@lakchote lakchote self-assigned this Sep 19, 2024
@lakchote lakchote moved this to Polish in [#whatsnext] #expense Sep 19, 2024
@lakchote lakchote added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Sep 19, 2024
Copy link

melvin-bot bot commented Sep 19, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021836682788967357264

@melvin-bot melvin-bot bot changed the title [Search v2.3] Add the bookmark icon for Saved searches on mobile [$250] [Search v2.3] Add the bookmark icon for Saved searches on mobile Sep 19, 2024
Copy link

melvin-bot bot commented Sep 19, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 19, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add the bookmark icon for Saved searches on mobile

What is the root cause of that problem?

On mobile, we display the saved search query that we're viewing here

And we don't add icon field for the saved searches item

const savedSearchItems = savedSearchesMenuItems.map((item) => ({

What changes do you think we should make in order to solve the problem?

We should remove this item here because it will be included in the savedSearchItems below or we can exclude the viewing saved search in savedSearchItems

Add icon field as Expensicons.BookMark here, we can also add other icon config field if it needs

icon: Expensicons.Bookmark,

const savedSearchItems = savedSearchesMenuItems.map((item) => ({

What alternative solutions did you explore? (Optional)

@thesahindia
Copy link
Member

thesahindia commented Sep 19, 2024

We should remove this item here because it will be included in the savedSearchItems below or we can exclude the viewing saved search in savedSearchItems

The solution for adding icon is fine but I am not sure I understand what you said above. Could you explain it?

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 19, 2024

If we're viewing a saved search, we shouldn't add the custom query option to the bottom modal. Example: In the imagine below, the type:expense status:all currency:ALL AMD shouldn't be shown

As the ref comment here mentioned, we shouldn't display the viewing saved search query in popoverMenuItems or savedSearchItems

@thesahindia
Copy link
Member

Got it!

@thesahindia
Copy link
Member

@mkzie2's proposal looks fine to me!

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Sep 19, 2024

Current assignee @lakchote is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@lakchote
Copy link
Contributor Author

@mkzie2's proposal LGTM.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

📣 @mkzie2 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 20, 2024

Will raise the PR soon.

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 23, 2024

@thesahindia The PR is ready #49583.

@luacmartins luacmartins self-assigned this Sep 23, 2024
@luacmartins luacmartins moved this from Polish to Done in [#whatsnext] #expense Sep 26, 2024
@melvin-bot melvin-bot bot removed the Weekly KSv2 label Oct 16, 2024
@melvin-bot melvin-bot bot added the Monthly KSv2 label Oct 16, 2024
Copy link

melvin-bot bot commented Oct 16, 2024

This issue has not been updated in over 15 days. @lakchote, @luacmartins, @thesahindia, @mkzie2 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!

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 17, 2024

@lakchote Can we process payment here?

@lakchote lakchote added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 18, 2024
Copy link

melvin-bot bot commented Oct 18, 2024

Triggered auto assignment to @VictoriaExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Monthly KSv2 labels Oct 18, 2024
@lakchote
Copy link
Contributor Author

@VictoriaExpensify please process payment for @mkzie2, thanks!

@luacmartins luacmartins changed the title [$250] [Search v2.3] Add the bookmark icon for Saved searches on mobile [HOLD for production 2024-10-07][$250] [Search v2.3] Add the bookmark icon for Saved searches on mobile Oct 21, 2024
@luacmartins luacmartins added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Oct 21, 2024
Copy link

melvin-bot bot commented Oct 21, 2024

@lakchote, @luacmartins, @VictoriaExpensify, @thesahindia, @mkzie2 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@luacmartins
Copy link
Contributor

Waiting on payment

@VictoriaExpensify
Copy link
Contributor

Hey @mkzie2 - can you please accept the job offer and I'll organise payment
https://www.upwork.com/nx/wm/offer/104530588

@melvin-bot melvin-bot bot removed the Overdue label Oct 22, 2024
@mkzie2
Copy link
Contributor

mkzie2 commented Oct 22, 2024

@VictoriaExpensify Did it thank you

@thesahindia
Copy link
Member

Hey @VictoriaExpensify, I am eligible for the C+ compensation. I will request it on new dot. Could you please add the payment summary here? I will need it to get the money request approved.

@VictoriaExpensify
Copy link
Contributor

Payment summary:
Contributor: @mkzie2 - Paid $250 via Upwork
Contributor+: @thesahindia - owed $250 via NewDot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
Status: Done
Development

No branches or pull requests

5 participants