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

Add tooltips on product's combination image #7808

Merged
merged 2 commits into from Jun 22, 2017

Conversation

@kompilorb
Copy link
Contributor

kompilorb commented Apr 20, 2017

Questions Answers
Branch? develop
Description? When adding images to product with different names and trying to assign them to combinations, it's very difficult to assign to a specific combination because the name of the image doesn't show up when the mouse hovers over it
Type? new feature
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-2730
How to test? Go to product declination edition, and with the mouse go over the image for see the name of this.
@kompilorb kompilorb changed the title BO: Add tooltips on product's declinaison image Add tooltips on product's declinaison image Apr 24, 2017
@vincentbz vincentbz added this to the 1.7.1.2 milestone Apr 24, 2017
@maximebiloe maximebiloe changed the title Add tooltips on product's declinaison image Add tooltips on product's combination image May 9, 2017
@maximebiloe maximebiloe modified the milestones: 1.7.2.0, 1.7.1.2 May 9, 2017
@maximebiloe maximebiloe changed the base branch from 1.7.1.x to develop May 9, 2017
@maximebiloe

This comment has been minimized.

Copy link
Contributor

maximebiloe commented May 9, 2017

As it's indicated in the table, it's a new feature, so I move the PR to develop.
Also, I'm not a big fan of JavaScript in the twig files as we have JS files dedicated to the product page.
Is there a specific reason of doing this here?

@kompilorb kompilorb force-pushed the kompilorb:1.7.1.x_images branch 2 times, most recently from 51941b7 to 2e72a36 May 31, 2017
@kompilorb

This comment has been minimized.

Copy link
Contributor Author

kompilorb commented May 31, 2017

@maximebiloe i move the js code in the files dedicated to the combination product page.

Copy link
Member

eternoendless left a comment

Shouldn't this be done when creating the img element instead?

@@ -217,6 +217,13 @@ var combinations = (function() {
number.text(countSelectedProducts() + '/' + allProductCombination);
});

$(function() {

This comment has been minimized.

Copy link
@eternoendless

eternoendless Jun 6, 2017

Member

Why the $(function() { ... }) wrapper ?

This comment has been minimized.

Copy link
@kompilorb

kompilorb Jun 6, 2017

Author Contributor

$(function() { ... }); is just jQuery short-hand for $(document).ready(function() { ... });
It's why the function is wrapper.

@@ -217,6 +217,13 @@ var combinations = (function() {
number.text(countSelectedProducts() + '/' + allProductCombination);
});

$(function() {

This comment has been minimized.

Copy link
@eternoendless

eternoendless Jun 6, 2017

Member

For future reference, please add a comment stating what this piece of code is supposed to do

For example: "Add title on product's combination image"

This comment has been minimized.

Copy link
@kompilorb

kompilorb Jun 6, 2017

Author Contributor

Ok, I add a comment.

@@ -217,6 +217,13 @@ var combinations = (function() {
number.text(countSelectedProducts() + '/' + allProductCombination);
});

$(function() {
$('#combination_form_' + contentElem.attr('data')).find("img").each(function() {
title = $(this).attr('src').split('/').reverse()[0];

This comment has been minimized.

Copy link
@eternoendless

eternoendless Jun 6, 2017

Member

You can also .pop() to pick the last element from your array (instead of .reverse()[0]).

This comment has been minimized.

Copy link
@kompilorb

kompilorb Jun 6, 2017

Author Contributor

OK, it's the same but i modify the function.

@kompilorb kompilorb force-pushed the kompilorb:1.7.1.x_images branch from 2e72a36 to fff6164 Jun 6, 2017
@Quetzacoalt91 Quetzacoalt91 dismissed eternoendless’s stale review Jun 22, 2017

Changes applied

@Quetzacoalt91 Quetzacoalt91 merged commit be5647e into PrestaShop:develop Jun 22, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Jun 22, 2017

Thank you @kompilorb

nihco2 pushed a commit to nihco2/PrestaShop that referenced this pull request Jun 22, 2017
Add tooltips on product's combination image
@kompilorb kompilorb deleted the kompilorb:1.7.1.x_images branch Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.