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

[IMP] web: improve kanban tooltip #24777

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@msh-odoo
Copy link
Contributor

msh-odoo commented May 17, 2018

1) Remove the total record tooltip from kanban group.
2) Remove progressbar help from kanban view.
3) Improvements in testcase since we have removed total record tooltip.

Task:https://www.odoo.com/web#id=1845459&action=333&active_id=131&model=project.task&view_type=form&menu_id=4720
Pad: https://pad.odoo.com/p/r.7e13ec256c6c38a54652df07556bd309

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@msh-odoo msh-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch May 17, 2018

@msh-odoo

This comment has been minimized.

Copy link
Contributor Author

msh-odoo commented May 18, 2018

LGTM

@C3POdoo C3POdoo added the RD label May 22, 2018

@vas-odoo vas-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch 3 times, most recently Aug 20, 2018

@vas-odoo vas-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch to c18cbde Sep 7, 2018

@vas-odoo vas-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch from e581c5f Sep 17, 2018

@robodoo robodoo removed the CI 🤖 label Sep 17, 2018

@vas-odoo vas-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch Sep 17, 2018

@msh-odoo msh-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch Sep 18, 2018

@vas-odoo vas-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch Sep 20, 2018

@tivisse tivisse force-pushed the odoo-dev:master-kanban-tooltip-vas branch Feb 7, 2019

@aab-odoo
Copy link
Contributor

aab-odoo left a comment

The diff in web should be in a separated commit. I also made a comment on the diff. Besides that, from a technical point, it's ok.

Note that i only reviewed the web part.

addons/web/static/src/js/views/kanban/kanban_column.js Outdated
return (data.tooltipData && data.tooltipData[field] && "<div>" + help + "<br>" + data.tooltipData[field] + "</div>") || '';
if (data.tooltipData && data.tooltipData[field]) {
help = help ? help + "</br>" : help;
data.tooltipData[field] = (!help && !_.values(data.tooltipData).includes(false)) ? data.tooltipData[field] + "<div class='dropdown-divider' role='separator' />" : data.tooltipData[field];

This comment has been minimized.

@aab-odoo

aab-odoo Feb 15, 2019

Contributor

includes is ES6 (not supported by ie11), so you can't use it. You can use indexOf instead.
Also, this line is way too long and hard to read/understand, i would try to re-write it to make it more readable.

This comment has been minimized.

@msh-odoo

msh-odoo Feb 18, 2019

Author Contributor

@aab-odoo It seems that something went wrong when @tivisse did rebase, I removed includes when reviewed it, I rebased branch again, you can check it now, I will make separate commit for web part, Thanks

@msh-odoo msh-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch 2 times, most recently Feb 18, 2019

@msh-odoo msh-odoo force-pushed the odoo-dev:master-kanban-tooltip-vas branch 2 times, most recently Feb 18, 2019

vas-odoo added some commits May 11, 2018

[IMP] web: improve kanban tooltip
1) Remove the total record tooltip from kanban group.
2) Remove progressbar help from kanban view.
3) Improvements in testcase since we have removed total record tooltip.
[IMP] *: Add progress bar on kanban views based on activities
Purpose
=======

If the activities are available on a model (i.e. inherits mail.activity.mixin),
add a progress bar based on the activities if nothing is defined yet.

@tivisse tivisse force-pushed the odoo-dev:master-kanban-tooltip-vas branch to 5f0452f Feb 19, 2019

@tivisse

This comment has been minimized.

Copy link
Contributor

tivisse commented Feb 19, 2019

@tivisse

This comment has been minimized.

Copy link
Contributor

tivisse commented Feb 19, 2019

@robodoo rebase-ff

@robodoo robodoo added the r+ 👌 label Feb 19, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 19, 2019

Merge method set to rebase and fast-forward

@robodoo robodoo added the CI 🤖 label Feb 19, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 19, 2019

Linked pull request(s) odoo/enterprise#2581 not ready. Linked PRs are not staged until all of them are ready.

robodoo pushed a commit that referenced this pull request Feb 19, 2019

[IMP] *: Add progress bar on kanban views based on activities
Purpose
=======

If the activities are available on a model (i.e. inherits mail.activity.mixin),
add a progress bar based on the activities if nothing is defined yet.

closes #24777

robodoo pushed a commit that referenced this pull request Feb 19, 2019

[IMP] *: Add progress bar on kanban views based on activities
Purpose
=======

If the activities are available on a model (i.e. inherits mail.activity.mixin),
add a progress bar based on the activities if nothing is defined yet.

closes #24777

@robodoo robodoo added merged 🎉 and removed merging 👷 labels Feb 19, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 19, 2019

Merged, thanks!

@robodoo robodoo closed this Feb 19, 2019

@tivisse tivisse deleted the odoo-dev:master-kanban-tooltip-vas branch Feb 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.