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 Payment 2024-09-19][Search v2.1] Search Results Page Header Cleanup #47254

Closed
rayane-djouah opened this issue Aug 12, 2024 · 31 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 NewFeature Something to build that is a new item.

Comments

@rayane-djouah
Copy link
Contributor

rayane-djouah commented Aug 12, 2024

This issue is a follow-up to the discussions in these links:
#46736 (comment)
#46962 (comment)

Problem:
For some filters, IDs are currently being displayed instead of names.

Objective:
We need to determine and agree on the appropriate information to show to the user.

Screenshots for Reference:

Screenshot 2024-08-12 at 6 27 31 PM

image

image

cc @luacmartins @SzymczakJ @WojtekBoman @Kicu

Issue OwnerCurrent Issue Owner: @289Adam289
@rayane-djouah rayane-djouah added Daily KSv2 NewFeature Something to build that is a new item. labels Aug 12, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

Triggered auto assignment to @puneetlath (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 12, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Aug 12, 2024

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

@rayane-djouah
Copy link
Contributor Author

Do we need to hold this until all filters are done?

@melvin-bot melvin-bot bot removed the Overdue label Aug 21, 2024
@luacmartins
Copy link
Contributor

This is lower priority, so let's focus on filters first.

@puneetlath
Copy link
Contributor

Can one of y'all link the issue that I should put this on hold for?

@luacmartins luacmartins changed the title [Search v2.1] Search Results Page Header Cleanup [HOLD #46030] [Search v2.1] Search Results Page Header Cleanup Aug 21, 2024
@luacmartins
Copy link
Contributor

Holding for #46030

@Kicu
Copy link
Contributor

Kicu commented Aug 22, 2024

May I suggest we also hold for this updated after v2.1 - #47787
Reason is that after this PR the policyID value will behave slightly differently and be part of the parsed AST, and what we are showing to the user are values coming from the parsed AST. policyID should be part of the discussion I think

@luacmartins luacmartins changed the title [HOLD #46030] [Search v2.1] Search Results Page Header Cleanup [HOLD #46030 and 47787] [Search v2.1] Search Results Page Header Cleanup Aug 22, 2024
@luacmartins
Copy link
Contributor

Done! Added to the hold list

@Kicu
Copy link
Contributor

Kicu commented Aug 29, 2024

Hold on #47787 can be removed - PR was merged

@puneetlath puneetlath changed the title [HOLD #46030 and 47787] [Search v2.1] Search Results Page Header Cleanup [HOLD #46030] [Search v2.1] Search Results Page Header Cleanup Aug 29, 2024
@luacmartins luacmartins changed the title [HOLD #46030] [Search v2.1] Search Results Page Header Cleanup [Search v2.1] Search Results Page Header Cleanup Aug 29, 2024
@289Adam289
Copy link
Contributor

As @rayane-djouah said in #47254 (comment) I think we should agree on the appropriate information to show to the user. Currently, I am taking an approach similar to how filters are displayed in AdvancedFiltersPage and just want to confirm if that's the right way. I also have two questions:

  1. Do we still agree on using whitespace as a delimiter
  2. Do we need type:${type} status:${status} part of header title given the contextual filters

@Kicu
Copy link
Contributor

Kicu commented Sep 4, 2024

Re 2. I believe we do not want to display type and status. These will be highlighted on their respective status/type bars and buttons.

@luacmartins
Copy link
Contributor

Correct, we don't need to show type/status. As for 1, yes space should still be a delimiter

@rayane-djouah
Copy link
Contributor Author

I foresee a potential issue with type. Specifically, we deselect the LHN item (search type) if custom query, and on mobile, we display the header title in the dropdown menu button without the type.

@luacmartins
Copy link
Contributor

@rayane-djouah I'm not sure that I understand the issue. Could you please clarify?

@289Adam289
Copy link
Contributor

How to handle translations? For example filters:has, is, expenseType are predefined values pickers and their values are translated into Spanish. We can translate them in header but in that case I think keywords such as expenseType should be translated as well.

@luacmartins
Copy link
Contributor

@289Adam289 I don't think we should translate the filters in the header for now. The query syntax doesn't support translations yet, so we'd be educating users on a feature that's not available yet.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 6, 2024
@rayane-djouah
Copy link
Contributor Author

Note

The production deploy automation failed: This should be on [HOLD for Payment 2024-09-19] according to #49025 production deploy checklist, confirmed in #48538 (comment)

@puneetlath puneetlath changed the title [Search v2.1] Search Results Page Header Cleanup [HOLD for Payment 2024-09-19][Search v2.1] Search Results Page Header Cleanup Sep 17, 2024
@luacmartins luacmartins added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Sep 17, 2024
@luacmartins luacmartins added Daily KSv2 and removed Weekly KSv2 labels Sep 17, 2024
@melvin-bot melvin-bot bot removed the Overdue label Sep 17, 2024
@luacmartins luacmartins moved this from Polish to Done in [#whatsnext] #expense Sep 17, 2024
Copy link

melvin-bot bot commented Sep 23, 2024

@puneetlath, @dannymcclain, @luacmartins, @rayane-djouah, @289Adam289 Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Sep 23, 2024
@puneetlath
Copy link
Contributor

@rayane-djouah it looks like you're the only one due payment here, correct? And it was for one PR?

@rayane-djouah
Copy link
Contributor Author

@puneetlath Correct!

@puneetlath
Copy link
Contributor

Great! Offer here: https://www.upwork.com/nx/wm/offer/104093170. Please ping me on this issue when you've accepted.

@rayane-djouah
Copy link
Contributor Author

@puneetlath Offer accepted, thank you!

@puneetlath
Copy link
Contributor

Great. All handled. Thanks everyone!

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
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 Daily KSv2 NewFeature Something to build that is a new item.
Projects
Status: Done
Development

No branches or pull requests

6 participants