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

Added search by columns. #23

Merged
merged 3 commits into from
May 7, 2019
Merged

Added search by columns. #23

merged 3 commits into from
May 7, 2019

Conversation

rmgpinto
Copy link
Contributor

@rmgpinto rmgpinto commented May 3, 2019

@drewbanin I've added the ability to search by columns.

Changes:

  • project_service.js: refactored fuzzySearchObj function to allow search by string or object (json tree). On a later PR I can add search by array which will allow to search for model tags as well.
  • search.html: changed html to reflect layout shown in screenshot (see below). The layout has a Show More... to expand the results of more than 3 columns, just like at the end of the model search results.
  • search.js: added columnFilter and limitColumns functions to execute the search logic.

Screenshot: link

@drewbanin
Copy link
Contributor

drewbanin commented May 3, 2019

@rmgpinto this is amazing!! Thanks for opening this PR! I think you're the first external contributor to the docs website :)

I'd love to get this merged - let me play around with this today and get back to you early next week!

@drewbanin
Copy link
Contributor

Hey @rmgpinto - I just merged this PR to add CI to the dbt-docs repo: #24

Can you pull master and merge it into your branch then re-push? We'll be able to see a sample build with your changes!

@rmgpinto
Copy link
Contributor Author

rmgpinto commented May 4, 2019

@drewbanin all done.

Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

@rmgpinto this is really great!

My one bit of UX feedback is that the columns: list should be contained within the clickable search result div. In playing around with this, it looks like the Show X more link might be hard to click when the user hovers over a particular search result. Was that your thinking here?

Screen Shot 2019-05-06 at 10 16 19 AM

Otherwise, the code looks really wonderful and I'm super impressed with this PR! Let me know what you think about the "columns:" div placement :)

@rmgpinto
Copy link
Contributor Author

rmgpinto commented May 7, 2019

Thanks for the suggestion @drewbanin, I've reflected that on the layout and it looks better now :)

I had to add $event.stopPropagation() to prevent the click on Show more... columns to navigate to the model page.

Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

@rmgpinto it was a true pleasure to receive and review this PR -- thanks for your contribution to dbt Docs! I'm going to merge this, and we're going to slip it into the 0.13.1 release of dbt, to be released later this week :)

Thanks, and great work!

@drewbanin drewbanin merged commit 60eff58 into dbt-labs:master May 7, 2019
@rmgpinto
Copy link
Contributor Author

rmgpinto commented May 7, 2019

Thanks @drewbanin!

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.

2 participants