Move bootstrap modal to toolbar to fix swiftype search

lexakami commented 5 years ago

Move search modal implementation from .org to global toolbar
Remove old global search implementation
Add jquery dependency

Used in .org https://github.com/silverstripeltd/silverstripe.org/pull/143 and docs https://github.com/silverstripe/doc.silverstripe.org/pull/188

robbieaverill commented 5 years ago

Summary

Changes look good, but compatibility testing with the other websites:

silverstripe.org

Works well, nice one.

userhelp.silverstripe.org

Using PR: https://github.com/silverstripe/userhelp.silverstripe.org/pull/87

Clicking search gives me a console error:

(index):1301 Uncaught TypeError: Cannot read property 'classList' of null
at elAddClass ((index):1301)
at HTMLAnchorElement.<anonymous> ((index):1447)

docs.silverstripe.org

Using PR: https://github.com/silverstripe/doc.silverstripe.org/pull/188

Not able to get the global toolbar running for this site on my local environment =/ anyway, it should behave the same way as userhelp.

addons.silverstripe.org

You'll need to consider making a PR for this into the SilverStripe 4 branch as well. addons.silverstripe.org is an example of a site that uses SilverStripe 4, so would also need to be updated if this is deployed to silverstripe.org.

api.silverstripe.org

There are also some global search references in https://github.com/silverstripe/api.silverstripe.org/blob/master/conf/themes/silverstripe/layout/layout.twig#L31-L57 which might need to be tweaked, although that website doesn't use this module, it copies some of the markup over. It's not a SilverStripe website.

lexakami commented 5 years ago

@robbieaverill WIP PR for userhelp https://github.com/silverstripe/userhelp.silverstripe.org/pull/87, this will just need to be updated once toolbar is merged

Will take a look at api.silverstripe.org and creating a PR to merge into master as well 👍

lexakami commented 5 years ago

Additional PR for merging to master (SS4) https://github.com/silverstripe/silverstripe-globaltoolbar/pull/50 - is this what you meant?

Updated addons.silverstripe.org with search modal (draft PR awaiting toolbar merge) https://github.com/silverstripe/addons.silverstripe.org/pull/247

silverstripe / silverstripe-globaltoolbar