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

docs: Add stub articles for search guides #10772

Merged
merged 6 commits into from
Jan 25, 2019
Merged

docs: Add stub articles for search guides #10772

merged 6 commits into from
Jan 25, 2019

Conversation

m-allanson
Copy link
Contributor

Description

Update the existing search guide to act as an overview for more specific guides.

I've also updated the [[guidelist]] handler to remove the hardcoded paths (cc @jlengstorf)

screenshot 2019-01-02 at 14 25 47

Related Issues

Refs #10113

@m-allanson m-allanson requested a review from a team January 2, 2019 14:27
@m-allanson m-allanson requested a review from a team as a code owner January 2, 2019 14:27
@jonniebigodes
Copy link

@m-allanson i'm working on documentating how to search with reactivesearch. I can expand the document i'm producing for both of elasticunr and js-search also. Algolia for me it's not something i really not that confortable, so i'll refrain from expanding that stub and probably fumble fingering it.

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

This looks great! Left a comment on a code style thing, feel free to take or leave it!

@@ -15,9 +15,26 @@ import docsHierarchy from "../data/sidebars/doc-links.yaml"
// Find the guides in the sidebar YAML.
const guides = docsHierarchy.find(group => group.title === `Guides`).items

// Search through the guides and their children to find the first item that
// matches the given slug
const findBySlug = (guides, slug) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks better, but I wonder if we could make it less side-effecty with a .find and perhaps a flatten? Here's a start:

const flatten = arr => arr.reduce((merged, item) => merged.concat(Array.isArray(item.items) ? flatten(item.items) : item), [])

const findBySlug = (guides, slug) => {
  return flatten(guides)
    .find(item => item.link === slug)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Presuming we have a structure like (for guides):

const guides = [
    {
      link: '/test'
    },
    {
      items: [
        {
          link: '/test2'
        }
      ]
    }
  ]

this seems to work just fine!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a little play around with this but I don't think flatten is right here, as we want to grab the child items from the item with the matching slug. I rejigged some other bits while playing around with this though, thank you 👍

@DSchau DSchau assigned shannonbux and unassigned shannonbux Jan 4, 2019
www/src/data/sidebars/doc-links.yaml Outdated Show resolved Hide resolved
www/src/data/sidebars/doc-links.yaml Outdated Show resolved Hide resolved
www/src/data/sidebars/doc-links.yaml Outdated Show resolved Hide resolved
@wardpeet wardpeet added the status: awaiting author response Additional information has been requested from the author label Jan 16, 2019
m-allanson and others added 4 commits January 25, 2019 00:03
Co-Authored-By: m-allanson <michael.allanson@gmail.com>
Co-Authored-By: m-allanson <michael.allanson@gmail.com>
Co-Authored-By: m-allanson <michael.allanson@gmail.com>
@m-allanson m-allanson merged commit 8f0215d into master Jan 25, 2019
@m-allanson m-allanson deleted the search-guides branch January 25, 2019 00:31
@jonniebigodes
Copy link

ok, so with this merge now my pull request #11030 is basically throwing merge conflicts with what is now the master. If you don't mind, a little feedback would be appreciated on this matter

@m-allanson
Copy link
Contributor Author

Sorry I missed your comment earlier, I've fixed up the merge conflicts on your branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting author response Additional information has been requested from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants