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

Advanced search #2199

Closed
wants to merge 18 commits into from
Closed

Advanced search #2199

wants to merge 18 commits into from

Conversation

huyxgit
Copy link

@huyxgit huyxgit commented Apr 21, 2021

Screen Shot 2021-04-23 at 2 12 48 AM

@huyxgit
Copy link
Author

huyxgit commented Apr 21, 2021

@humphd this question is a bit further when this advanced search component completed: should we wire up this search dialog to the searchButton on Nav bar instead of redirecting to search-page right away?

@humphd
Copy link
Contributor

humphd commented Apr 22, 2021

@huynguyez good question. At some point (later) we might want to have a way to search from the main page without moving over to this page. For now, let's have a way to get to the advanced search from the search page, but not have that be the default.

@huyxgit
Copy link
Author

huyxgit commented Apr 23, 2021

@HyperTHD

@izhuravlev
Copy link
Contributor

@huynguyez do you need help? I can hop on a call in about 2 hours and work on this so we can merge it.

@huyxgit
Copy link
Author

huyxgit commented Apr 23, 2021

@izhuravlev sure, still problems with the dialog, state. What time?

Copy link
Contributor

@HyperTHD HyperTHD left a comment

Choose a reason for hiding this comment

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

Just a preliminary review. There's stuff other stuff I need to look at but this is a few stuff I caught

src/web/src/components/SearchBar.tsx Outdated Show resolved Hide resolved
src/web/src/components/SearchBar.tsx Outdated Show resolved Hide resolved
src/web/src/components/AdvancedSearchDialog.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@HyperTHD HyperTHD left a comment

Choose a reason for hiding this comment

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

You're probably going to have to update the SearchProvider so that you can access a state toggle value similar to SearchHelp. Look at how SearchHelp's toggle value is implemented in SearchProvider.tsx for reference

src/web/src/components/AdvancedSearchDialog.tsx Outdated Show resolved Hide resolved
@izhuravlev
Copy link
Contributor

I can't input text into the search bar - sorry for the delay, we can stay on the call after the meeting and work on it till it's done

@huyxgit
Copy link
Author

huyxgit commented Apr 24, 2021

@HyperTHD I tried to store the state in global but it still same. any idea?

@huyxgit
Copy link
Author

huyxgit commented Apr 25, 2021

Not work when I store state in Context (search provider). But using states in local component is working as expected. Obviously this should be optimized later

Copy link
Contributor

@LuigiZaccagnini LuigiZaccagnini left a comment

Choose a reason for hiding this comment

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

Small change.

dialogContent: {
background: '#eeeeee',
height: '300px',
color: 'black',
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using black you can use #000000 instead.

@AmasiaNalbandian AmasiaNalbandian mentioned this pull request Dec 8, 2021
8 tasks
@TueeNguyen
Copy link
Contributor

Hi @huyxgit, any update on this work?

@AmasiaNalbandian
Copy link
Contributor

There is work in progress on the branch right here:
https://github.com/AmasiaNalbandian/telescope/tree/issue-2110-advancedsearch

I have cherry picked the beginning code for the modal, and have made more progress by implementing the backend for the advanced search from the PR that was added here: #2617

I am going to close this PR as it is abandoned, and I have picked up the issue.

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.

7 participants