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

Enable Gitbook search plugin #1897

Closed
wants to merge 1 commit into from
Closed

Enable Gitbook search plugin #1897

wants to merge 1 commit into from

Conversation

filoxo
Copy link

@filoxo filoxo commented Nov 13, 2018

Resolves #1892 by enabling the Gitbook search plugin.

There is a risk that upgrading to the latest version of gitbook-cli may break building the docs (see GitbookIO/gitbook-cli#74) but as I understand the issue, since this won't be changing the version of gitbook (only the gitbook-cli) I'm hopeful that the previously mentioned issue won't arise here.

@filoxo
Copy link
Author

filoxo commented Dec 18, 2018

Is there something I can do to inspire greater confidence on deploying this change?

@ljharb
Copy link
Member

ljharb commented Dec 18, 2018

@filoxo sorry for the delay here, i haven't had time to get to this. I'll test it and try to merge it asap.

@ljharb
Copy link
Member

ljharb commented Dec 18, 2018

I've rebased this branch and I'm comparing the built docs.

One difference is illustrated here:

before after
before after

@filoxo
Copy link
Author

filoxo commented Dec 19, 2018

Huh, the menu thing is interesting. The documentation for collapsible-menu actually says that having the menu items closed is the expected behavior.

https://github.com/rtCamp/gitbook-plugin-collapsible-menu#how-this-work

First it hides all pages, and then using .active css class show's its parent and grandparent nodes (chapters).

I also noticed it behaves the same way (all menu items collapsed) when navigating away from the "Introduction" page and back on the live site.

I'll keep digging around and see if there's an option or config that will allow all menu items to be open by default, but would removing collapsed-menu plugin be a better solution if you prefer not having the menu items closed?

@ljharb
Copy link
Member

ljharb commented Dec 19, 2018

I don't mind the initial state so much, but i can't even click on the menus to expand them. Whatever it takes to make that happen would be great :-)

@filoxo
Copy link
Author

filoxo commented Dec 19, 2018

I found that even if search isn't enabled, gitbook includes the search-lunr plugin by default (in the .gitbooks folder) and attempts to set gitbooks.search engine, but search isn't defined so it throws this error.

jQuery.Deferred exception: Cannot read property 'getEngine' of undefined TypeError: Cannot read property 'getEngine' of undefined
    at Object.<anonymous> (https://airbnb.io/enzyme/gitbook/gitbook-plugin-lunr/search-lunr.js:54:37)
    at Object.dispatch (https://airbnb.io/enzyme/gitbook/gitbook.js:2:19320)
    at Object.m.handle (https://airbnb.io/enzyme/gitbook/gitbook.js:2:17339)
    at Object.trigger (https://airbnb.io/enzyme/gitbook/gitbook.js:3:8538)
    at Object.<anonymous> (https://airbnb.io/enzyme/gitbook/gitbook.js:3:9059)
    at Function.each (https://airbnb.io/enzyme/gitbook/gitbook.js:1:15309)
    at de.fn.init.each (https://airbnb.io/enzyme/gitbook/gitbook.js:1:13477)
    at de.fn.init.trigger (https://airbnb.io/enzyme/gitbook/gitbook.js:3:9034)
    at Object.r [as hasChanged] (https://airbnb.io/enzyme/gitbook/gitbook.js:4:8170)
    at https://airbnb.io/enzyme/:2240:26 undefined

After that error is thrown, the collapsible-menu plugin doesn't seem to be executed until the next page change event. I manually fixed this and ran the old version of the docs locally, and the menu collapses just like the new version does.
That's just a long winded way of saying that the menu was broken without search having been enabled and now that it is, its working as intended.

But definitely, the menu should be clickable! I've tested this locally on latest Chrome, Firefox, and Safari on Mac, and on Windows 10 using Edge and IE11; and all of them work for me. Could you share which browser/version/platform you're using? As well as any console errors. The collapse-menu plugin uses jQuery so I wonder if some other error is being thrown and causing other plugins to fail.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2018

Safari on Mac - but I'm using file://, and it seems gitbook makes cross-origin requests for some reason?

When I use a local http server, things work, but it'd still be nice if the menus were collapsible/expandable without navigation - and the more I look at it, the more I think being initially expanded is the right choice.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2018

For the record, you're right - on the current site, TypeError: undefined is not an object (evaluating 'gitbook.search.getEngine') seems to be the reason they're initially expanded, but other pages show them collapsed.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2018

Also, on the new site, i see twitter/facebook/share icons - let's remove those :-)

@ljharb
Copy link
Member

ljharb commented Dec 19, 2018

The CSS also seems a bit different; the old site is a bit more compact (in a good way).

@filoxo
Copy link
Author

filoxo commented Dec 19, 2018

I'm going to go ahead and decline this PR then, since it's introducing more changes than expected. I might try getting the old version working and see if I'm just dumb and can workaround the issues I'd noted in #1892 to avoid upgrading gitbook-cli.

Enabling search will cause all menu items to be closed on initial navigation because the collapsible-menu plugin will work correctly. Unfortunately there is no config/setting that will achieve what you like without forking it, which I'd rather not do because I won't maintain it. I just wanted search-ability on the docs site.

@filoxo filoxo closed this Dec 19, 2018
@ljharb
Copy link
Member

ljharb commented Dec 20, 2018

I’ll reopen this then and see what i can figure out.

@ljharb ljharb reopened this Dec 20, 2018
@ljharb ljharb force-pushed the master branch 3 times, most recently from 40cc703 to 0a17404 Compare April 12, 2019 23:05
@filoxo
Copy link
Author

filoxo commented Jan 13, 2020

Closing my own stale PRs.

@filoxo filoxo closed this Jan 13, 2020
@ljharb
Copy link
Member

ljharb commented Jan 13, 2020

I'm still hoping to get back to this; hopefully it's ok that it remains open.

@ljharb ljharb reopened this Jan 13, 2020
@ljharb ljharb force-pushed the master branch 4 times, most recently from 2227326 to 0d5ead7 Compare December 21, 2020 07:46
@filoxo filoxo closed this Nov 5, 2023
@ljharb
Copy link
Member

ljharb commented Nov 5, 2023

@filoxo unfortunately by deleting the fork this PR is unrecoverable :-( i wish you hadn't done that.

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.

Can't search https://airbnb.io/enzyme/
2 participants