Skip to content

added code for promotedSuggestions & for special characters on hover … - #110

Open
yuvakishore-dommaraju wants to merge 2 commits into
masterfrom
newFeatures
Open

added code for promotedSuggestions & for special characters on hover …#110
yuvakishore-dommaraju wants to merge 2 commits into
masterfrom
newFeatures

Conversation

@yuvakishore-dommaraju

@yuvakishore-dommaraju yuvakishore-dommaraju commented Sep 26, 2019

Copy link
Copy Markdown

added code for promotedSuggestions & for special characters on hover of suggestions

While creating autoSuggest Url: — do I need to add following params as default or not ? (I have not added for the following Items)
--> variants=true
--> variants.fields= fieldValues {{variant_products_fields ? variant_products_fields : '*’}}
--> filter=-unbxd_isSearchable_uFilter:"false"&filter=-unbxd_isSearchable_filter:”false"
--> not made changes for relevantDocument in processfilteredPopularProducts & processPopularProducts functions

Comment thread unbxdAutosuggest.js
this.$input.val($(lis[this.activeRow]).data('value'));
if (this.options.filtered && this.activeColumn === 0) {
var dataValue = $(lis[this.activeRow]).attr('data-value') ? $(lis[this.activeRow]).attr('data-value') : '';
var dataValue = $(lis[this.activeRow]).attr('data-value') ? $(lis[this.activeRow]).attr('data-value').replace('amp;', '') : '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sanitize this in catalog.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Integration done from on-boarding portal so we are not able to add functionality from onboarding console

Comment thread unbxdAutosuggest.js
var cmpld = ""
if (self.options.popularProducts.viewMore && self.options.popularProducts.viewMore.enabled) {
cmpld = Handlebars.compile(self.preparefilteredPopularProducts() + self.options.popularProducts.viewMore.tpl);
if (this.options.popularProducts.viewMore && this.options.popularProducts.viewMore.enabled) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we replaced self with this, we are making sure of scope of options by defining self

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

for self this is not assigned in this function, which is leading for errors like popularProducts are undefined

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is fixed in master as of now

Comment thread unbxdAutosuggest.js
if (typeof this.options.onItemSelect == "function" && data.type !== "POPULAR_PRODUCTS_FILTERED") {
this.options.onItemSelect.call(this, data, this.currentResults[data.type][parseInt(data['index'])]._original, e);
if (typeof this.options.onItemSelect == "function" && data.type !== "POPULAR_PRODUCTS_FILTERED" && data.type !== "POPULAR_PRODUCTS") {
this.options.onItemSelect.call(this, data, this.currentResults['COMBINE_RESULTS'][parseInt(data['index'])]._original, e);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need COMBINE_RESULTS

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added sorting functionality as well, there are customers who look for sorting irrespective of suggestions since, we are making changes in library all things can be tested once so that next time we no need to change library simply we can handle from config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorting is added if infields are not present in suggestions, let's enhance on that?
Line 1597

Comment thread unbxdAutosuggest.js Outdated
+'</span></li>'
+'{{/if}}'

// +'{{#if isInField}}'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we have this comment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removing

Comment thread unbxdAutosuggest.js Outdated
}
}

if(self.currentResults['POPULAR_PRODUCTS'].length == 0 && !isMobile.any()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need this.

@yuvakishore-dommaraju yuvakishore-dommaraju Sep 26, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removing, seems like it's customised for ikea

Comment thread unbxdAutosuggest.js Outdated
uniqueInfields = [],
uniqueSuggestions = [];

var sorted_list = [],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add description why we need this.

@ganeshparsads

Copy link
Copy Markdown
Contributor

@yuvakishore-dommaraju why is there a question on PR description.

Comment thread unbxdAutosuggest.js Outdated
//use unbxd scope and add a version for autosuggest
window.Unbxd = window.Unbxd || {};
Unbxd.autosuggestVersion = "1.0.1";
Unbxd.autosuggestVersion = "1.0.1";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove space.

Comment thread unbxdAutosuggest.js Outdated
return v1 === v2 ? options.fn(this) : options.inverse(this);
});

Handlebars.registerHelper('removeDots', function(id){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need this helper?

Comment thread unbxdAutosuggest.js Outdated
, popularProducts: {
count: 2
, price: true
, fields:['*']

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we need this?

Comment thread unbxdAutosuggest.js
var query = dataValue + (dataFiltername != '' ? ':' + dataFiltername + ':' + dataFiltervalue : '')
// updating product header while hovering on suggestions
if (this.options.filtered && this.options.popularProducts.header) {
if (this.options && this.options.popularProducts.header) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we removed filtered

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

parent level If condition is checking for this.options.filtered so here If we make sure options & popularProducts pass the condition that will be enough

@yuvakishore-dommaraju

yuvakishore-dommaraju commented Sep 26, 2019

Copy link
Copy Markdown
Author

@yuvakishore-dommaraju why is there a question on PR description.

I'm not sure adding those parameters will impact existing clients or not who are using this library so, checking. If these changes (mentioned in comments) will not impact any clients I can change add it to the code @ganeshparsads

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants