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

Fix svg confict in algolia vender #173

Merged
merged 4 commits into from
Mar 15, 2018
Merged

Conversation

sli1989
Copy link
Collaborator

@sli1989 sli1989 commented Mar 15, 2018

In former file, all svg-related element in blog would be blocked leading to the conflict between algolia and svgs. Related issue.

+svg path {
+ display: none;
+}

In this PR, more exact element are found and blocked using algolia CDN , in this way, other svg-related element can be display usually. More related issues about the missing of SVG can be solved.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 15, 2018

Tested on all schemes? Please, attach screens with tests in all schemes (4 screens).

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

I find a strange phenomenon. With or without the above bugfix using CDN,

  1. the first click on the search box, works fine. but the box can be closed only with clicking the close button instead of clicking outside of the area of the search box.
  2. the 2nd click on the search box, there is a overflow where the search box can't be displayed.

_20180315151536
qq 20180315151506

@ivan-nginx
Copy link
Member

Try to disable sidebar, bug still exists?
On all schemes this bug can be reproduced?

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

All scheme with local lib or vender CDN, the bug is still exist. Need further investment.

@ivan-nginx
Copy link
Member

@sli1989 reproduce steps, please. I'll join this pull as remote and trying to test.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

  1. latest release
  2. algolia_search: enable: true
  3. add commit, or not. It's not the priority.
  4. using git clone https://github.com/theme-next/theme-next-algolia-instant-search source/lib/algolia-instant-search in cd themes/next or set vendor CDN
vendors:
  algolia_instant_js: https://cdn.jsdelivr.net/npm/instantsearch.js@2.4.1/dist/instantsearch.js
  algolia_instant_css: https://cdn.jsdelivr.net/npm/instantsearch.js@2.4.1/dist/instantsearch.min.css
  1. hexo clean && hexo g && hexo s

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 15, 2018

U test on your domen or u have any subdomen(s) for tests? If yes, can i see live demo with this?

P.S. I'm just not use Algolia and need to register this under new test domen|subdomain: algolia-search.js?v=6.0.6:11 Algolia Settings are invalid.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

I haven't push them to the blog, I would do it if i failed to find the bug. I am trying to test on V604.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 15, 2018

@sli1989 can u create subdomain for tests, something like test.saili.science?

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

Deployed for now in http://saili.science/

@ivan-nginx
Copy link
Member

So, local_search is turned off for now? Where i can see full NexT config?

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

Just leave the local_search alone, i haven't uninstall the hexo-generator-searchdb. The full files can be found in gitlab.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 15, 2018

i haven't uninstall the hexo-generator-searchdb

No need to do this. Need just to disable local_search option in config.

I see this config for now as default:

algolia_search:
  enable: false
  hits:
    per_page: 10
  labels:
    input_placeholder: Search for Posts
    hits_empty: "We didn't find any results for the search: ${query}"
    hits_stats: "${hits} results found in ${time} ms"

# Local search
# Dependencies: https://github.com/theme-next/hexo-generator-searchdb
local_search:
  enable: true

But if u want to use Algolia search instead of Local search (u no need 2 search boxes at once, right?), i think need to disable local search:

algolia_search:
  enable: true
  hits:
    per_page: 10
  labels:
    input_placeholder: Search for Posts
    hits_empty: "We didn't find any results for the search: ${query}"
    hits_stats: "${hits} results found in ${time} ms"

# Local search
# Dependencies: https://github.com/theme-next/hexo-generator-searchdb
local_search:
  enable: false

Try it.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

emm. i haven't update the repo. I did all like this.

I forgot the local_search: enable: false has enable option....

@ivan-nginx
Copy link
Member

So, u talking about what it's clean repo, right? But in your source code (view-source:http://saili.science/) i see source of local_search option (search by .local-search-pop-overlay) and Algolia search too (search by algolia-search.js). So, i think what both Local and Algolia searches was enabled during this deploy.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

Yes, you are right. Solved, but the search box can be closed only with clicking the close button instead of clicking outside of the area.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 15, 2018

So, bug was in both Algolia and Local searches enabled? I think need to add console warning or something like this on this situation.


just closed only with clicking the close button instead of clicking outside of the area of the search box

And this was added only for Local search in this pull. In Algolia search this closed by outside type wasn't added. But, anyone can add it, if he want. 😄

Initially, local search was created and modified by @flashlab and then was greatest modifications from @uchuhimo.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

I have no svg-related plugin to run test. Just perform same as before to avoid display redundant element when using CDN vendor. Need screenshots?

@ivan-nginx
Copy link
Member

No need screenshots. Just explain what's not work for now?

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 15, 2018

ADDED IN PR CONTENT.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 15, 2018

«Update branch», «Squash and merge».

@sli1989 sli1989 merged commit 0972b8c into theme-next:master Mar 15, 2018
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
* Fix svg confict in algolia vender
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.

2 participants