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

Master better search in settings qmo #28094

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants

@robodoo robodoo added the CI 🤖 label Oct 24, 2018

@xmo-odoo

This comment has been minimized.

Collaborator

xmo-odoo commented Oct 24, 2018

UPDATE ME

4c1

@C3POdoo C3POdoo added the RD label Oct 24, 2018

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 2, 2018

[IMP] base: srch applied on descr + better order
[FIX] base: Fix QUnit breadcrumb test fail

Improves searching in settings.
Instead of the actual fixed order, top results should be about the module
you are in.

closes : odoo#28094

@robodoo robodoo removed the CI 🤖 label Nov 2, 2018

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 2, 2018

[IMP] base: srch applied on descr + better order
[FIX] base: Fix QUnit breadcrumb test fail

Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

closes : odoo#28094

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 2, 2018

[IMP] base: srch applied on descr + better order
[FIX] base: Fix QUnit breadcrumb test fail

Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : odoo#28094

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 2, 2018

[IMP] base: srch applied on descr + better order
Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : odoo#28094

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 2, 2018

[IMP] base: srch applied on descr + better order
Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : odoo#28094

@robodoo robodoo added the CI 🤖 label Nov 2, 2018

@qmo-odoo

This comment has been minimized.

qmo-odoo commented Nov 2, 2018

Purpose :
Allows users to not only search on settings name but also on their description.
Improves the results order : Top result should be about the selected module.

Current behavior before PR:
Users can only search on settings name and the results order is fixed no matter the module you're in.

Desired behavior after PR:
Users can search on descriptions and top results are about the module they selected in the left panel

Task : https://www.odoo.com/web?debug#id=1893252&action=333&active_id=965&model=project.task&view_type=form&menu_id=4720

@ged-odoo

Not a big fan, but the initial code is not good...

Also, there are no tests. Please add at least one test

@@ -9,6 +9,7 @@ var FormController = require('web.FormController');
var FormRenderer = require('web.FormRenderer');
var view_registry = require('web.view_registry');

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

useless noise

@@ -21,17 +22,17 @@ var BaseSettingRenderer = FormRenderer.extend({
this._super.apply(this, arguments);
this.activeView = false;
this.activeTab = false;
this.initial_order;

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

this.initialTabElements = undefined;

},

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

noise, remove this

start: function () {
this._super.apply(this, arguments);
if (config.device.isMobile) {
core.bus.on("DOM_updated", this, function () {
this._moveToTab(this.currentIndex || this._currentAppIndex());
});
}
}

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

noise, remove this

@@ -236,7 +248,7 @@ var BaseSettingRenderer = FormRenderer.extend({
_onKeyUpSearch: function (event) {
this.searchText = this.searchInput.val();
this.activeTab.removeClass('selected');
// this.activeTab.removeClass('selected');

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

remove this

@@ -199,6 +200,17 @@ var BaseSettingRenderer = FormRenderer.extend({
* @param {int} index
*/
_moveToTab: function (index) {
//Used to reorder the settings after a search and a tab switch
//Thought it would be better for the UX if the order stayed the same as the one on the left panel
if(!this.initial_order && this.$el.find('.settings')[0]){

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

you use 3 times the this.$el.find('.settings')[0] expression. Also a lookup in the DOM is kind of expensive. You can simply do this:

var settingsNode = this.$('.settings')[0];
if (!this.initialTabElements && settingsNode[0]) {...
if(!this.initial_order && this.$el.find('.settings')[0]){
//Converts an HtmlCollection into an array
this.initial_order = Array.prototype.slice.call(this.$el.find('.settings')[0].children);
}

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

else on same line as }

this.initial_order = Array.prototype.slice.call(this.$el.find('.settings')[0].children);
}
else{
var current_order = this.$el.find('.settings');

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor
this.$('.settings')
    .empty()
    .append(this.initialTabElements)
@@ -319,7 +337,7 @@ var BaseSettingRenderer = FormRenderer.extend({
if (self.inVisibleCount !== resultSetting.length) {
module.settingView.find('.settingSearchHeader').removeClass('o_hidden');
module.settingView.removeClass('o_hidden');
}
}

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

remove this

@@ -299,13 +311,19 @@ var BaseSettingRenderer = FormRenderer.extend({
_searchSetting: function () {
var self = this;
this.count = 0;
var settings = $(this.$el.find('.settings')[0].children)

This comment has been minimized.

@ged-odoo

ged-odoo Nov 6, 2018

Contributor

; at the end of this (and the next) lines

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 8, 2018

[IMP] base: srch applied on descr + better order
Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : odoo#28094

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Nov 8, 2018

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 12, 2018

[IMP] base: srch applied on descr + better order
Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : odoo#28094

@robodoo robodoo removed the CI 🤖 label Nov 12, 2018

qmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Nov 12, 2018

[IMP] base: srch applied on descr + better order
Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : odoo#28094

@robodoo robodoo added the CI 🤖 label Nov 12, 2018

[IMP] base: srch applied on descr + better order
Allows users to not only search on settings name but on their description too.
Improves search results relevance :
Instead of the actual fixed order, top results should be about the module
selected in the left panel

Task : #1893252
closes : #28094

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Nov 13, 2018

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