-
Notifications
You must be signed in to change notification settings - Fork 2k
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 search services #1012
Merged
Merged
Refactor search services #1012
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
5b36c83
Refactor search services
stevenjoezhang d50439c
Fix QA
stevenjoezhang 13d4061
Code style update
stevenjoezhang 977c21b
Update
stevenjoezhang f18d8f8
Fix QA
stevenjoezhang c1c3541
Update
stevenjoezhang 50eba45
Update style
stevenjoezhang 05b4dab
Swiftype
stevenjoezhang 56d4f49
Fix QA
stevenjoezhang cf0e1c2
Update layout
stevenjoezhang 954e754
Update local-search.js
stevenjoezhang c598a90
Update
stevenjoezhang cb35df7
Update
stevenjoezhang 3f85832
Preload
stevenjoezhang cae63ea
Fix QA
stevenjoezhang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,5 @@ | ||
{% if theme.algolia_search.enable %} | ||
{% include 'algolia-search.swig' %} | ||
{% elif theme.swiftype_key %} | ||
{% include 'swiftype.swig' %} | ||
{% elif theme.local_search.enable %} | ||
{% include 'localsearch.swig' %} | ||
{% endif %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,15 @@ | ||
{% if theme.algolia_search.enable %} | ||
|
||
{# S: Include Algolia instantsearch.js library #} | ||
{% set algolia_instant_css = url_for(theme.vendors._internal + '/algolia-instant-search/instantsearch.min.css') %} | ||
{% if theme.vendors.algolia_instant_css %} | ||
{% set algolia_instant_css = theme.vendors.algolia_instant_css %} | ||
{% endif %} | ||
<link rel="stylesheet" href="{{ algolia_instant_css }}"/> | ||
|
||
{% set algolia_instant_js = url_for(theme.vendors._internal + '/algolia-instant-search/instantsearch.min.js') %} | ||
{% if theme.vendors.algolia_instant_js %} | ||
{% set algolia_instant_js = theme.vendors.algolia_instant_js %} | ||
{% endif %} | ||
<script src="{{ algolia_instant_js }}"></script> | ||
{# E: Include Algolia instantsearch.js library #} | ||
{# S: Include Algolia instantsearch.js library #} | ||
{% set algolia_instant_css = url_for(theme.vendors._internal + '/algolia-instant-search/instantsearch.min.css') %} | ||
{% if theme.vendors.algolia_instant_css %} | ||
{% set algolia_instant_css = theme.vendors.algolia_instant_css %} | ||
{% endif %} | ||
<link rel="stylesheet" href="{{ algolia_instant_css }}"/> | ||
|
||
<script src="{{ url_for(theme.js) }}/algolia-search.js?v={{ version }}"></script> | ||
{% set algolia_instant_js = url_for(theme.vendors._internal + '/algolia-instant-search/instantsearch.min.js') %} | ||
{% if theme.vendors.algolia_instant_js %} | ||
{% set algolia_instant_js = theme.vendors.algolia_instant_js %} | ||
{% endif %} | ||
<script src="{{ algolia_instant_js }}"></script> | ||
{# E: Include Algolia instantsearch.js library #} | ||
|
||
<script src="{{ url_for(theme.js) }}/algolia-search.js?v={{ version }}"></script> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,7 @@ | ||
{% include 'localsearch.swig' %} | ||
{% include 'algolia-search.swig' %} | ||
{% if theme.algolia_search.enable %} | ||
{% include 'algolia-search.swig' %} | ||
{% elif theme.swiftype_key %} | ||
{% include 'swiftype.swig' %} | ||
{% elif theme.local_search.enable %} | ||
{% include 'localsearch.swig' %} | ||
{% endif %} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Move
layout/_partials/search/*
tolayout/_third-party/search/*
?I don't think it's necessary to have two layouts
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.
jQuery is loaded at the end of the body. Local search depends on jQuery. Put the script here will cause an
$
not defined error. Maybe we can open another PR to move jQuery to<head>
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.
Don't do this.
我的意思是将原本在菜单那的搜索弹出框布局移到底部,与脚本合并,弹出框与其所在的位置没有关联,两个文件变一个,更便于维护
I mean to move the search popup layout that was originally in the menu to the bottom, merge with the script, the popup is not associated with its location, and the two files become one, which is easier to maintain.
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.
但是将DOM和
<script>
分离更加优美。在layout
中,<script>
只应存在于_scripts
和_third-party
中,各种DOM存在于_partials
中。就像评论系统,使用inject.comment向评论区写入DOM,使用inject.bodyend写入脚本。它们应该是分开的。还是需要开一个新的PR,处理这样的情况。
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.
这是因为
comment
的位置比较尴尬,不能在bodyEnd也不是postBodyEnd 😂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.
另外一个问题是各种share,也是同时存在DOM元素和脚本。真蛋疼..
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.
share完全不知道怎么搞,在我的基于next的主题里是全删了,反正我也不用,哈哈😄
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.
所以这咋办呀
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.
不知道,那就暂时不管share咯,以后想到怎么处理了在弄。
这是我的搜索重构 jiangtj/hexo-theme-cake#11
加menu注入点,试了把localsearch提取出去,等GitHub包管理出了,可以这样搞啊,反正localsearch本来就要装个hexo插件:joy:
等你这个合并,我修改下插件直接CDN引用localsearch.js,美滋滋