-
Notifications
You must be signed in to change notification settings - Fork 913
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
Split Navigation and Search pages #1352
Conversation
Work so far splits the pages and fixes the couple of internal links that the split broke. There is also the possibility of it breaking external links, though sadly I'm pretty sure you can't redirect to/from anchors.... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. See inline for a few nits.
LGTM aside from the nits identified above. The link checker seems happy, so that's good. (The issue of title font color on the homepage is fixed via #1467, FYI.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
Resolve merge issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Search page is missing changes that were applied to the "Site search options" section since this PR was created: the most obvious are the updates to the config file names, but there could be more. @LisaFC - did you recopy the "Site search options" section from @HEAD
?
I thought I did but possibly it got lost in the merge. Let me add those in again. Thanks for spotting! |
Hang on, I've realized they're in the intro, which I didn't change because I rewrote it. Let me fix the file names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! (I've confirmed that no updates were lost.)
Fixes #1047
Preview: https://deploy-preview-1352--docsydocs.netlify.app/docs/adding-content/search/