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

Fixed reset behavior on Product form #6004

Merged

Conversation

@mickaelandrieu
Copy link
Member

commented Aug 12, 2016

Questions Answers
Branch? develop
Description? We can't use button with type reset in product page, except if we want to reset the complete global form, this contribution is about reset the specific price rule form only.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
How to test? ping @vincentbz :- )

Important guidelines

@vincentbz

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2016

Tested and ok, thanks !

@julienbourdeau

View changes

src/PrestaShopBundle/Form/Admin/Product/ProductSpecificPrice.php Outdated
'label' => $this->translator->trans('Cancel', array(), 'Admin.Actions'),
'attr' => array('class' => 'btn-default-outline js-cancel'),
));
/*
* note: ResetType can't be used because the product page is wrapped inside a big form

This comment has been minimized.

Copy link
@julienbourdeau

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Aug 12, 2016

Author Member

🐈

This comment has been minimized.

Copy link
@Shudrum

Shudrum Aug 16, 2016

Contributor

Same here. And note: is a little bit redundant on a comment ^^

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Aug 16, 2016

Author Member

This is perfectly valid in PHP. More, this is a multiline comment and apply this comment
on top of this class make the chances to be read close to 0.

I agree on the redundancy of note, removed 👍

@mickaelandrieu mickaelandrieu force-pushed the mickaelandrieu:reset-specific-price-form branch 2 times, most recently Aug 12, 2016

@Shudrum

View changes

admin-dev/themes/default/js/bundle/product/form.js Outdated
@@ -610,6 +610,13 @@ var specificPrices = (function() {
});
}

function resetForm() {
/**

This comment has been minimized.

Copy link
@Shudrum

Shudrum Aug 16, 2016

Contributor

Not the right comment type, should be //

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Aug 16, 2016

Author Member

This is intended. From my own experience, multi lines comment are more visible. This comment is VERY important, regarding the impact on the back office.

This comment has been minimized.

Copy link
@Shudrum

Shudrum Aug 16, 2016

Contributor

https://github.com/airbnb/javascript#comments

Multiline comments will only be used for methods & functions descriptions. Not for this.

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Aug 16, 2016

Author Member

As you like sir.

@mickaelandrieu mickaelandrieu force-pushed the mickaelandrieu:reset-specific-price-form branch Aug 16, 2016

@mickaelandrieu

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2016

@Shudrum comments adressed, thanks for review !

@mickaelandrieu mickaelandrieu force-pushed the mickaelandrieu:reset-specific-price-form branch Aug 17, 2016

@mickaelandrieu

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2016

@Shudrum rebased :)

@mickaelandrieu mickaelandrieu force-pushed the mickaelandrieu:reset-specific-price-form branch 2 times, most recently Aug 17, 2016

@mickaelandrieu mickaelandrieu force-pushed the mickaelandrieu:reset-specific-price-form branch to fb0843b Aug 17, 2016

@maximebiloe maximebiloe merged commit 6e0cfe8 into PrestaShop:develop Aug 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maximebiloe maximebiloe deleted the mickaelandrieu:reset-specific-price-form branch Aug 18, 2016

@maximebiloe

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2016

Thank you Mickaël!

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