-
Notifications
You must be signed in to change notification settings - Fork 72
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
Change search #1249
Change search #1249
Conversation
@matiasbenary is attempting to deploy a commit to the Near Developer Console Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@shelegdmitriy we might need some help with css here, is that something you could help with? otherwise I'll see to dust off my css foo |
Sure I can help you here! Let me check the PR and I'll respond in a while |
@matiasbenary may I ask you to fix CI/Lint please? |
I fixed it |
src/utils/angoliaSearchApi.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo -> algoliaSearchApi.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we decided to show this search input with dropdown for desktop devices only? I'd propose to stick to the current implementation: mobile only have a search icon which leads to search page. If you wish to improve this later - create a task. cc @gagdiez |
Hey @matiasbenary and @gagdiez. I see you did a lot of progress here. There is a room for improvements and I would like to help you but may I ask you to fix this issue 1 please? |
Also, @matiasbenary ping me please after you fix issue 1 please so I could pull your branch to make some additional changes, ok? |
I think the issue 1 is because we are soft-loading the page through a router, and not hard-refreshing the page. This keeps the state, and confuses the VM sometimes @matiasbenary |
@shelegdmitriy while I was not able to reproduce the error that you and @matiasbenary have, and I see that Matias was able to reproduce it in the current version of dev.near.org I think it is related to another issue, which is how we handle components and the VM in general: #1264 I would suggest to leave this for another time, as it is an already existing bug |
7db3137
to
4890cd7
Compare
Hey @gagdiez @matiasbenary @charleslavon. I believe I added everything I'd like to this PR so feel free to review this and leave your questions. One thing I want to highlight is new FYI: a lot of components are already implemented here and there was a preview page for all of them but I can't find it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@race-of-sloths I summon you! |
🙁 This repository is not a part of the Race of SlothsPlease ask the maintainer to contact us to join the race! What is the Race of Sloths
Race of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow For contributors:
For maintainers:
Feel free to check our website for additional details! |
That is done:
The search works for docs and for catalog.
When you click on docs it takes you inside the page.
Within apps when you click on the title it takes you where it belongs, you can also click on the tag.
Missing:
Missing components, I don't quite understand the filters I was using.
Separate in component and improve the styles.
Missing the full view of search when you search in mobile.
When you click to close
Paginated and see all
Order the code