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

✨Implemented Fuzzy Search through an API on the Dev Dashboard #19676

Merged
merged 15 commits into from Dec 19, 2018

Conversation

torch2424
Copy link
Contributor

@torch2424 torch2424 commented Dec 6, 2018

This implements fuzzy searching of the current directory using a new REST API for the dashboard, amp-list, amp-mustache, and amp-bind.

  • Creates the new api/ directory for all dashboard/api/* requests. Starting with /api/api.js
  • Creates a GET /listing api route. Takes a path and search query param.
  • Rewrites the file list to use amp list, and populate this list by calling the GET /listing api route.
  • Searching is done using amp-bind and changing the search query param for our amp list src.
  • Search input field is debounced, searches after delay, or after Enter Keypress (Good UX 😄 )

See examples below. Tested API using Postman. Tested UI in all Chrome Canary, Firefox, and Safari.

fuzzyfindingdemo

screen shot 2018-12-05 at 4 21 03 pm

screen shot 2018-12-05 at 4 21 12 pm

@alanorozco
Copy link
Member

Thanks for this, looks awesome!

Before I take a more technical look, would you mind styling the list so that each row takes a constant amount of space? I see it jumping in size in the gif, which is rather jarring.

This might be due to amp-list requiring a fixed-size, but I'm not sure. I'm going to give it a go.

@torch2424
Copy link
Contributor Author

@alanorozco Fixed the column size! Was because I used justify-content: space-between. 😄 But flex-start doesn't look too bad either so yeah.

Also, I fixed all the linting errors 😄

@alanorozco
Copy link
Member

I meant the spacing between rows like here:

image

I'm also noticing that the rows don't take up the full-width:

image

@alanorozco
Copy link
Member

Two ideas:

  1. Instead of <div placeholder>Loading...</div> we can render the filelist. That way we don't get a flash of unloaded content! :)

  2. Fixed-height amp-list is causing a lot of issues. We can mitigate some of it by switching to layout: FLEX_ITEM and let the list take up the available window space with overflow: auto. Each individual row should take up a fixed height so the row spacing issue should be mitigated. In the future, we can use dynamic resizing

Also note that I made some small changes to this PR, so make sure to pull.

@torch2424
Copy link
Contributor Author

@alanorozco

Fixed tests, and presubmit errors. Also, made sure to pull down your commits and everything 😄

Instead of

Loading...
we can render the filelist. That way we don't get a flash of unloaded content! :)

I agree this is awesome! Since this PR is already kinda big, do you mind if I open an issue and implement this another time? Just to keep this PR size small.

Fixed-height amp-list is causing a lot of issues. We can mitigate some of it by switching to layout: FLEX_ITEM and let the list take up the available window space with overflow: auto. Each individual row should take up a fixed height so the row spacing issue should be mitigated. In the future, we can use dynamic resizing

I definitely agree that we should move to dynamic resizing for amp-list once that gets into PROD. But in the meantime, I Fixed the row height issue 🎉 :

screen shot 2018-12-07 at 7 05 11 pm

@alanorozco
Copy link
Member

@torch2424 Awesome! One more request, could you please make sure the row height is the same as before?

noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
…ject#19676)

* Created an API for dashboard, and a fuzzy search endpoint

* Got the client side of fuzzy searching working

* Got fuzzy searching working

* Still working on fuzzy finding

* Got debounced input

* Finished up fuzzy search for files

* Finished dashboard fuzzy finding

* Fixed the space between column styling

* Fixed linting errors

* Search box styling, structure

* naming

* Fixed huge rows, and tests

* Made the rows and columns the same sizes as they were originally
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants