Skip to content

Global search bar#690

Merged
mihaisc merged 8 commits into
feature/multinetworkfrom
feature/665-global_search_bar
Jul 6, 2021
Merged

Global search bar#690
mihaisc merged 8 commits into
feature/multinetworkfrom
feature/665-global_search_bar

Conversation

@mihaisc

@mihaisc mihaisc commented Jun 17, 2021

Copy link
Copy Markdown
Contributor

Handles #665
Highly WIP. Functionality is sort of there.

What search pattern should we follow?

A. Easier way, like amazon
image
image

B. More design work (on mobile)
image
image
image

Bonus feature : search should trigger on 'enter'

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>
@mihaisc mihaisc requested a review from kremalicious as a code owner June 17, 2021 10:17
@vercel

vercel Bot commented Jun 17, 2021

Copy link
Copy Markdown

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oceanprotocol/market/B3dsQKSuFNiQm3AXkHz2BnLpCMd8
✅ Preview: https://market-git-feature-665-globalsearchbar-oceanprotocol.vercel.app

@mihaisc mihaisc marked this pull request as draft June 17, 2021 10:18
@kremalicious

Copy link
Copy Markdown
Contributor

B would be so nice but that's a lot to refactor I guess. So from behavior standpoint would go with A, but visually combine the two where the search field starts collapsed as search icon button only, and as viewport gets bigger search field can become more visible like Rarible does. On mobile the search field upon interaction can still take over the whole bar on top of other elements, but then searching still only happens when user hits enter.

But then we need to make sure when user arrives on search page that searchbar is open by default

@mihaisc

mihaisc commented Jun 24, 2021

Copy link
Copy Markdown
Contributor Author

I think you managed to design a more complicated way 😆. Ok i'll try to get in in a place where just UI changes are needed

@mihaisc

mihaisc commented Jun 25, 2021

Copy link
Copy Markdown
Contributor Author

draft 1 fullscreen
image
and it actually scales , until a point
image

@mihaisc

mihaisc commented Jun 28, 2021

Copy link
Copy Markdown
Contributor Author

Some more work needed on the button and behaviour
search1

Signed-off-by: mihaisc <mihai@oceanprotocol.com>
Signed-off-by: mihaisc <mihai@oceanprotocol.com>
@mihaisc

mihaisc commented Jun 28, 2021

Copy link
Copy Markdown
Contributor Author

Needs more testing and the button interactions not that great.

@mihaisc mihaisc marked this pull request as ready for review June 29, 2021 11:51
@mihaisc

mihaisc commented Jun 29, 2021

Copy link
Copy Markdown
Contributor Author

There are bugs when searching by owner or tags (when you click on owner or tag in asset details) that will be fixed once we merge main in the multinetwork branch.
The button still needs some UI tweaks but it's beyond me

@jamiehewitt15

Copy link
Copy Markdown
Contributor

The styling is good. Only one small thing I would suggest is there should be more padding on the right of the History link.

So it changes from this:
history padding 0px 2021-06-30_16-50

To this:
history padding 25px 2021-06-30_16-50

@jamiehewitt15

Copy link
Copy Markdown
Contributor

It terms of the search functionality, I'm noticing a couple issues which I thought we had solved in: #641

  • The results don't include a partial match. For example, if you search "data union" you'll find that "dataunion.app" doesn't appear in the results
  • Searching with a trailing space effects the search results.

@mihaisc

mihaisc commented Jul 1, 2021

Copy link
Copy Markdown
Contributor Author

@jamiehewitt15 search fixes were not merged, so basically this is the old search with multiple bugs. Those will be merged in the multinetwork branch.
I'll add some padding where you suggested

@mihaisc

mihaisc commented Jul 2, 2021

Copy link
Copy Markdown
Contributor Author

Main is now merged.

The results don't include a partial match. For example, if you search "data union" you'll find that "dataunion.app" doesn't appear in the results

This was not fixed, requires more work, it's a bit more complicated.

@jamiehewitt15 jamiehewitt15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The functionality and styling look good to me. I just added a couple of minor comments on the code

Comment thread src/components/molecules/Menu.module.css Outdated
Comment thread src/components/molecules/SearchBar.module.css Outdated
@mihaisc

mihaisc commented Jul 6, 2021

Copy link
Copy Markdown
Contributor Author

Removed comments, should be ok now

@jamiehewitt15

Copy link
Copy Markdown
Contributor

Looks good, I have approved

@mihaisc mihaisc merged commit 3798d80 into feature/multinetwork Jul 6, 2021
@mihaisc mihaisc deleted the feature/665-global_search_bar branch July 6, 2021 11:55
@kremalicious kremalicious mentioned this pull request Jul 12, 2021
@kremalicious kremalicious added this to the Multi-Network milestone Jul 12, 2021
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