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

refactor(theme): do not render search bar when excludeFromSearchIndex is true #154

Merged
merged 2 commits into from
Dec 13, 2019

Conversation

emmenko
Copy link
Member

@emmenko emmenko commented Dec 12, 2019

Fixes #149

@vercel
Copy link

vercel bot commented Dec 12, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/commercetools/commercetools-docs-kit/ce3lm7sve
🌍 Preview: https://commercetools-docs-git-nm-exclude-search-bar-when-no-ind-8d9bb1.commercetools.now.sh

Copy link
Collaborator

@nkuehn nkuehn left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation, clean and straightforward.

I suggest to set up the tests in a way that leaves the search box in the visual regression tests. Now that component is not "VRTed" any more at all (and we'll have the same issue for the top nav once that's there.)

Could require separate configuration of the search bar visibility unless you can find a way to let the tests - or a part of them - run in a mode that is "Indexed" without deploying the test site as indexed.

@emmenko
Copy link
Member Author

emmenko commented Dec 12, 2019

Yeah ok, will take a look at it tomorrow

@emmenko emmenko force-pushed the nm-exclude-search-bar-when-no-index-site branch from d6f52c4 to c7b47e1 Compare December 13, 2019 09:26
@vercel vercel bot temporarily deployed to staging December 13, 2019 09:26 Inactive
@emmenko emmenko force-pushed the nm-exclude-search-bar-when-no-index-site branch from c7b47e1 to 14ab054 Compare December 13, 2019 09:33
@vercel vercel bot temporarily deployed to staging December 13, 2019 09:33 Inactive
@emmenko
Copy link
Member Author

emmenko commented Dec 13, 2019

Alright, it should be good to go now.

@emmenko emmenko force-pushed the nm-exclude-search-bar-when-no-index-site branch from 14ab054 to 9f1145d Compare December 13, 2019 09:44
@vercel vercel bot temporarily deployed to staging December 13, 2019 09:44 Inactive
@emmenko emmenko merged commit b38fe12 into master Dec 13, 2019
@emmenko emmenko deleted the nm-exclude-search-bar-when-no-index-site branch December 13, 2019 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default to not showing the on-site search box if the whole site is configured "Noindex"
3 participants