Skip to content
This repository was archived by the owner on Apr 4, 2022. It is now read-only.

Adding owner address useSearchingAnotherNetwork #785

Merged
merged 8 commits into from
Oct 28, 2021

Conversation

@henrypalacios henrypalacios self-assigned this Oct 21, 2021
@henrypalacios henrypalacios added app:Explorer Explorer App Protofire task to the protofire team labels Oct 21, 2021
@github-actions
Copy link

@henrypalacios henrypalacios marked this pull request as ready for review October 22, 2021 23:35
@elena-zh
Copy link

Changes LGTM.
But maybe we should use a bit more simple message 'Orders are detected in:' instead of 'But have been detected in:'
detected

Anyways, it is my personal opinion and should not affect merging this PR if it looks good to other team members.

Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Few comments regarding the code below.

Regarding the behaviour, working quite nice!

Adding to the text suggestions:

  1. When there are no orders at all
    "No orders found"
  2. When there's order in a network other than the one searched for:
No orders found on <strong>xDai</strong>.
However, found orders on:
  <a>Mainnet</a>

...etc, like you did

@alfetopito
Copy link
Contributor

We can get @biocom and @alongoni input on styling the message as well as @anxolin 's input on the behaviour

@henrypalacios henrypalacios force-pushed the 768-try-find-another-network branch from 076b2b4 to 5fd3b38 Compare October 26, 2021 23:02
@henrypalacios henrypalacios force-pushed the 768-try-find-another-network branch from 5fd3b38 to 4d188e1 Compare October 26, 2021 23:33
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Very nice!

Approved with a minor comment

Copy link
Contributor

@matextrem matextrem left a comment

Choose a reason for hiding this comment

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

LGTM 👌

Co-authored-by: Leandro Boscariol <[email protected]>
@alongoni alongoni self-requested a review October 28, 2021 13:43
Copy link
Contributor

@alongoni alongoni left a comment

Choose a reason for hiding this comment

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

Looks fine!

@alfetopito alfetopito added the Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds label Oct 28, 2021
@mergify mergify bot merged commit 88b36dd into develop Oct 28, 2021
@alfetopito alfetopito deleted the 768-try-find-another-network branch October 28, 2021 22:37
This was referenced Nov 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
app:Explorer Explorer App Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds Protofire task to the protofire team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hint the network the user is looking for orders
5 participants