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

Implement Algolia Instantsearch and autocomplete feature #246

Closed
wants to merge 13 commits into from

Conversation

@sandipbhuyan
Copy link
Collaborator

@sandipbhuyan sandipbhuyan commented Jul 3, 2018

Resolves #245

@sandipbhuyan sandipbhuyan requested a review from imphil Jul 3, 2018
@sandipbhuyan sandipbhuyan requested a review from recrsn Jul 3, 2018
@sandipbhuyan sandipbhuyan changed the title Algolia Instantsearch and autocomplete feature Implement Algolia Instantsearch and autocomplete feature Jul 3, 2018
@@ -45,3 +45,7 @@ parameters:
# Register at https://sentry.io
sentry_dsn: '{{ site_sentry_dsn }}'
sentry_env: '{{ site_sentry_env }}'

# Algolia Configuration
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

not required. We are still debating whether to move into Symfony flex or not

Loading

@@ -45,3 +45,7 @@ parameters:
# Register at https://sentry.io
sentry_dsn: '{{ site_sentry_dsn }}'
sentry_env: '{{ site_sentry_env }}'

# Algolia Configuration
env(ALGOLIA_APP_ID): '{{ site_algolia_app_id }}'
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

not required. see above

Loading

@@ -8,4 +8,4 @@
# Require confirmation of the email address of newly registered users.
# Requires a working mailer setup. Add SMTP credentials in
# secrets/dev-vagrant.secrets.yml before enabling this setting.
#site_user_email_confirmation_enabled: true
#site_user_email_confirmation_enabled: true
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

Configure your IDE to respect eol settings from .editorconfig. There must be a plugin available

Loading

*/
public function normalize(NormalizerInterface $serializer, $format = null, array $context = array())
{
return [
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

👍
This is a nice feature

Loading

@@ -124,6 +126,24 @@ public function __construct()
$this->members = new ArrayCollection();
}

/**
* Normalizes the object into an array of scalars|arrays.
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

Use @inheritdoc

Loading

@@ -323,6 +325,49 @@ public function __construct()
$this->classifications = new ArrayCollection();
}

/**
* Normalizes the object into an array of scalars|arrays.
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

Loading

$("#{{ search_query_form.q.vars.id }}").focus();
searchFunctions();
options = {
appId: 'ENTER_APPLICATION_ID',
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

You can transclude the site_algolia_app_id here using twig globals

https://stackoverflow.com/a/8360126/2328165

Loading

@@ -0,0 +1,171 @@
function algoliaAutocomplete() {
var client = algoliasearch('ENTER_APPLICATION_ID', 'ENTER_APPLICATION_KEY')
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

Possible approach:
Transclude Algolia settings into window.__algoliaConfig__ in a twig template and utilize it by window.__algoliaConfig__.applicationId

<script>
window.__algoliaConfig__ = {
  applicationId: "{{ algolia_app_id }}"
  // ... more settings
}
</script>

Loading

var organization = client.initIndex('organization');
var user = client.initIndex('user');

autocomplete('#search-form-input', {debug:true}, [
Copy link
Collaborator

@recrsn recrsn Jul 4, 2018

Choose a reason for hiding this comment

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

Make configurable through a parameter

Loading

@sandipbhuyan sandipbhuyan force-pushed the algolia-search branch 4 times, most recently from a7baec5 to a974348 Jul 9, 2018
Copy link
Contributor

@imphil imphil left a comment

Thanks @sandipbhuyan! I had an initial look at the code but didn't run it yet, I'll do that later today and get back with more comments.

Loading

@@ -49,3 +49,4 @@ parameters:
# Algolia Configuration
env(ALGOLIA_APP_ID): '{{ site_algolia_app_id }}'
env(ALGOLIA_API_KEY): '{{ site_algolia_api_key }}'
ALGOLIA_SEARCH_KEY: '{{ site_algolia_search_key }}'
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

No env() needed on that one? Why? Can we keep the two consistent?

Loading

Copy link
Collaborator

@recrsn recrsn Jul 11, 2018

Choose a reason for hiding this comment

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

env() is used to substitute for a missing env var, where it is used in getenv. This is supposed to be used directly on the front end.

Loading


# Assetic Configuration
assetic:
debug: "%kernel.debug%"
use_controller: false
bundles:
- LibrecoresProjectRepoBundle
- LibrecoresSiteBundle
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

How is this change related to the PR? If it's not related please open a separate PR to keep this one focused.

Loading

Copy link
Collaborator Author

@sandipbhuyan sandipbhuyan Jul 10, 2018

Choose a reason for hiding this comment

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

The algolia autocomplete feature is going to be at home page which is at LibrecoresSiteBundle that's why I have added this bundle here

Loading

->getRepository('LibrecoresProjectRepoBundle:Organization')
->findByFragment($searchQuery->getQ());
}
// If searchType is null
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

... then what? How about "search projects by default" (or remove the comment altogether)

Loading

</span>
</div>
<hr/>
{% endverbatim %}
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

The indentation of this block got messed up a bit.

Loading

$(function() {
$("#{{ search_query_form.q.vars.id }}").focus();
});
{# do not use autofocus attribute to work around FF bug 712130 (FOUC) #}
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

is this comment still relevant?

Loading

<script src="{{ asset_url }}"></script>
{% endjavascripts %}
{% endblock %}

{% block pagejs %}
{# do not use autofocus attribute to work around FF bug 712130 (FOUC) #}
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

Place this comment closer to the code it actually describes, please.

Loading

<h2><a href="/{{ name }}">{{{ _highlightResult.displayName.value }}}</a></h2>
<div>{{{ _highlightResult.name.value }}}</div>
<hr/>
{% endverbatim %}
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

the indentation got messed up.

Loading

joined on {{{ createdAt.date }}}
</i></div>
<hr/>
{% endverbatim %}
Copy link
Contributor

@imphil imphil Jul 10, 2018

Choose a reason for hiding this comment

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

the indentation got messed up.

Loading

recrsn
recrsn approved these changes Jul 12, 2018
@recrsn
Copy link
Collaborator

@recrsn recrsn commented Jul 12, 2018

Barring a few indent errors, this is good to go!

Loading

…ces settings configuration files

The categoric hierarchy level is indexed under project entity and being sent to algolia, Now a user can configure or modify the algolia settings through the configuration files andimplement hierachichal search menu in instantsearch
@sandipbhuyan
Copy link
Collaborator Author

@sandipbhuyan sandipbhuyan commented Jul 31, 2018

#248 and #249 resolved

Loading

@imphil
Copy link
Contributor

@imphil imphil commented Aug 1, 2018

squashed and merged in 550622e

Loading

@imphil imphil closed this Aug 1, 2018
Improve the LibreCores.org in terms of discoverability automation moved this from Review to Done Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants