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

Fix search by order number in Navigator #5063

Merged
merged 7 commits into from
Jul 29, 2024
Merged

Conversation

Cloud11PL
Copy link
Member

What type of PR is this?

  • 💅 Refactor
  • 🌟 Feature
  • 🔥 Bug Fix
  • 🔩 Maintenance
  • 🛠 Workflow CI/CD changes

Related Issues or Documents

  • closes #

Usage Instructions, Screenshots, Recordings

CleanShot 2024-07-23 at 12 15 20

Have you written tests?

  • Yes!
  • No... here is why: Writing tests are mandatory, please replace this text with why test are not included in this PR

[Optional] Description

Copy link

changeset-bot bot commented Jul 23, 2024

🦋 Changeset detected

Latest commit: 7f345a1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot temporarily deployed to pr-5063 July 23, 2024 10:21 Destroyed
@Cloud11PL Cloud11PL marked this pull request as ready for review July 23, 2024 10:27
@Cloud11PL Cloud11PL requested a review from a team as a code owner July 23, 2024 10:27
@github-actions github-actions bot temporarily deployed to pr-5063 July 23, 2024 10:28 Destroyed
poulch
poulch previously approved these changes Jul 23, 2024
src/components/NavigatorSearch/modes/orders.ts Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pr-5063 July 23, 2024 19:06 Destroyed
@github-actions github-actions bot temporarily deployed to pr-5063 July 24, 2024 07:34 Destroyed
@Cloud11PL Cloud11PL requested a review from poulch July 24, 2024 07:39
poulch
poulch previously approved these changes Jul 24, 2024
@Cloud11PL Cloud11PL requested a review from poulch July 25, 2024 09:55
@github-actions github-actions bot temporarily deployed to pr-5063 July 25, 2024 09:56 Destroyed
poulch
poulch previously approved these changes Jul 25, 2024
Comment on lines 16 to 19
const { result } = renderHook(() => useQuickOrderSearch());

// Act
act(() => result.current[1]("1234"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { result } = renderHook(() => useQuickOrderSearch());
// Act
act(() => result.current[1]("1234"));
const { result } = renderHook(() => useQuickOrderSearch());
const [_, setQueryDebounced] = result.current
// Act
act(() => setQueryDebounced("1234"));

@Cloud11PL Cloud11PL requested a review from poulch July 25, 2024 12:09
@github-actions github-actions bot temporarily deployed to pr-5063 July 25, 2024 12:13 Destroyed
orderNumber,
}),
onClick: () => {
navigate(orderUrl(order.id));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't entries be a links? navigation based on onClick are hacky a bit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can improve it in the scope of the general Navigator improvements ticket (it's in backlog)

@Cloud11PL Cloud11PL merged commit e6be141 into main Jul 29, 2024
15 checks passed
@Cloud11PL Cloud11PL deleted the MERX-637-search-bar-order branch July 29, 2024 11:15
Cloud11PL added a commit that referenced this pull request Jul 30, 2024
* use search by order number

* fixes

* Update src/components/NavigatorSearch/modes/orders.ts

Co-authored-by: Paweł Chyła <[email protected]>

* lint

* add test for making search query

* improve test

---------

Co-authored-by: Paweł Chyła <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants