[WIP] Simplify product preferences #22471

JevgenijVisockij · 2020-12-17T14:22:42Z

Questions	Answers
Branch?	develop
Description?	Simplifies product preferences form
Type?	refacto
Category?	BO
BC breaks?	yes
Deprecations?	no
Fixed ticket?	Part of #16482
How to test?	Simplifying Configuration -> Shop Parameters -> Product Preferences. Everything must look the same aside for help now appearing under inputs instead as blue box, also some fields now will appear as required because they always were.

📓 BC Break
Backwards compatibility break introduced due to extension of TranslationAwareType by PageType and PaginationType. This means if any module extends those types they will get an exception upon upgrading to PS version containing changes in this PR.

This change is

prestonBot · 2020-12-17T14:23:14Z

This pull request seems to contain new translation strings. I have summarized them below to ease up review:

Admin.Notifications.Error
- %s is invalid. Please enter an integer greater than or equal to 0.
- The field is invalid. Please enter a positive integer.
Admin.Shopparameters.Feature
- Display the "%add_to_cart_label%" button when a product has attributes
Admin.Shopparameters.Help

(Note: this is an automated message, but answering it will reach a real human)

src/PrestaShopBundle/Form/Admin/Type/SwitchType.php

JevgenijVisockij · 2020-12-17T15:00:36Z

* button when a product has attributes

I fixed those but does it make sense to Display the "%add_to_cart_label%" button when a product has attributes to be in Admin.Shopparameters.Help when it's a label?

As of "Leave empty to disable." I changed it from "Leave empty to disable" and moved to Admin.Shopparameters.Help as it's part of help now. Is it okay?

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/StockType.php

Julievrz

* button when a product has attributes
I fixed those but does it make sense to Display the "%add_to_cart_label%" button when a product has attributes to be in Admin.Shopparameters.Help when it's a label?

As of "Leave empty to disable." I changed it from "Leave empty to disable" and moved to Admin.Shopparameters.Help as it's part of help now. Is it okay?

You are right, it is a label so it should be located as a feature. I am currently thinking of adapting the wording here because the label and the tooltip are not complementary. + the feature should be disabled by default and right now it is not. Also, this feature is not available for the classic theme and we're supposed to see a help text "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help (see the specs: https://github.com/PrestaShop/prestashop-specs/blob/master/back-office/shop-parameters/product-settings/product-settings-page.md)

@LouiseBonnard what do you think of the following wording?

Label: "Display the "Add to cart" button when a product has attributes" in Admin.Shopparameters.Feature
Tooltip: "By default, the "Add to cart" button is hidden when a product has attributes. You can choose to have it displayed in all cases." in Admin.Shopparameters.Help
Help text : "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/StockType.php

matks

Code is good 👍 waiting for wording decision

Julievrz · 2020-12-21T08:06:59Z

* button when a product has attributes
I fixed those but does it make sense to Display the "%add_to_cart_label%" button when a product has attributes to be in Admin.Shopparameters.Help when it's a label?
As of "Leave empty to disable." I changed it from "Leave empty to disable" and moved to Admin.Shopparameters.Help as it's part of help now. Is it okay?
You are right, it is a label so it should be located as a feature. I am currently thinking of adapting the wording here because the label and the tooltip are not complementary. + the feature should be disabled by default and right now it is not. Also, this feature is not available for the classic theme and we're supposed to see a help text "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help (see the specs: https://github.com/PrestaShop/prestashop-specs/blob/master/back-office/shop-parameters/product-settings/product-settings-page.md)

@LouiseBonnard what do you think of the following wording?

Label: "Display the "Add to cart" button when a product has attributes" in Admin.Shopparameters.Feature
Tooltip: "By default, the "Add to cart" button is hidden when a product has attributes. You can choose to have it displayed in all cases." in Admin.Shopparameters.Help
Help text : "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help

So I talked about it with @LouiseBonnard and I think we should just keep the help text:

"Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help"

and get rid of the tooltip (the feature can be specified in the user documentation if needed)

By default, the feature should be disabled.

JevgenijVisockij · 2020-12-22T14:39:33Z

* button when a product has attributes
I fixed those but does it make sense to Display the "%add_to_cart_label%" button when a product has attributes to be in Admin.Shopparameters.Help when it's a label?
As of "Leave empty to disable." I changed it from "Leave empty to disable" and moved to Admin.Shopparameters.Help as it's part of help now. Is it okay?
You are right, it is a label so it should be located as a feature. I am currently thinking of adapting the wording here because the label and the tooltip are not complementary. + the feature should be disabled by default and right now it is not. Also, this feature is not available for the classic theme and we're supposed to see a help text "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help (see the specs: https://github.com/PrestaShop/prestashop-specs/blob/master/back-office/shop-parameters/product-settings/product-settings-page.md)
@LouiseBonnard what do you think of the following wording?
Label: "Display the "Add to cart" button when a product has attributes" in Admin.Shopparameters.Feature
Tooltip: "By default, the "Add to cart" button is hidden when a product has attributes. You can choose to have it displayed in all cases." in Admin.Shopparameters.Help
Help text : "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help
So I talked about it with @LouiseBonnard and I think we should just keep the help text:

"Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help"

and get rid of the tooltip (the feature can be specified in the user documentation if needed)

By default, the feature should be disabled.

Added those changes just to confirm this is what you exepected this setting to look like?

install-dev/data/xml/configuration.xml

Julievrz · 2020-12-22T15:04:21Z

* button when a product has attributes
I fixed those but does it make sense to Display the "%add_to_cart_label%" button when a product has attributes to be in Admin.Shopparameters.Help when it's a label?
As of "Leave empty to disable." I changed it from "Leave empty to disable" and moved to Admin.Shopparameters.Help as it's part of help now. Is it okay?
You are right, it is a label so it should be located as a feature. I am currently thinking of adapting the wording here because the label and the tooltip are not complementary. + the feature should be disabled by default and right now it is not. Also, this feature is not available for the classic theme and we're supposed to see a help text "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help (see the specs: https://github.com/PrestaShop/prestashop-specs/blob/master/back-office/shop-parameters/product-settings/product-settings-page.md)
@LouiseBonnard what do you think of the following wording?
Label: "Display the "Add to cart" button when a product has attributes" in Admin.Shopparameters.Feature
Tooltip: "By default, the "Add to cart" button is hidden when a product has attributes. You can choose to have it displayed in all cases." in Admin.Shopparameters.Help
Help text : "Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help
So I talked about it with @LouiseBonnard and I think we should just keep the help text:
"Note that this setting does not work with the default theme anymore." in Admin.Shopparameters.Help"
and get rid of the tooltip (the feature can be specified in the user documentation if needed)
By default, the feature should be disabled.
Added those changes just to confirm this is what you exepected this setting to look like?

Yes, thank you !

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/GeneralType.php

Julievrz

The last modification before approving the wording for this PR. Thank you!

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/GeneralType.php

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/PageType.php

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/PaginationType.php

JevgenijVisockij · 2020-12-31T12:18:34Z

The last modification before approving the wording for this PR. Thank you!

Added those modifications, thanks!

Progi1984

A simple question about the configuration change

…ctPreferences/GeneralType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

…ctPreferences/PageType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

…ctPreferences/PaginationType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

src/PrestaShopBundle/Controller/Admin/Configure/ShopParameters/ProductPreferencesController.php

…/ProductPreferencesController.php Add then to a translation Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Progi1984 · 2021-03-31T13:09:53Z

...hopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/GeneralFormDataProvider.php

@@ -37,6 +41,10 @@
 */
 class GeneralFormDataProvider implements FormDataProviderInterface
 {
+    /** @todo change error to string once invoices PR merged  */


Which PR is involved ?

Progi1984 · 2021-03-31T13:13:07Z

@JevgenijVisockij Please fix PHPCSFixer

PierreRambaud · 2021-10-05T14:04:02Z

Hey @JevgenijVisockij
Are you still working on it? Should we close this one?
If no one is working on it, maybe someone from the community is able to finish the job in a new PR?

Kind regards

PierreRambaud · 2021-11-15T15:22:07Z

Hi @JevgenijVisockij ,

Since we had no news from you for more than 30 days, I'll close this pull request. Feel free to reopen or open another one if you think it's still relevant.

Thanks!

JevgenijVisockij requested a review from a team as a code owner December 17, 2020 14:22

prestonBot added develop Branch Refactoring Type: Refactoring labels Dec 17, 2020

prestonBot added the Waiting for wording Status: action required, waiting for wording label Dec 17, 2020

JevgenijVisockij changed the title ~~Simplify product preferences~~ [WIP] Simplify product preferences Dec 17, 2020

JevgenijVisockij force-pushed the feature/simplify-product-preferences branch from fa78468 to 9ef1afd Compare December 17, 2020 14:29

JevgenijVisockij changed the title ~~[WIP] Simplify product preferences~~ Simplify product preferences Dec 17, 2020

JevgenijVisockij commented Dec 17, 2020

View reviewed changes

src/PrestaShopBundle/Form/Admin/Type/SwitchType.php Show resolved Hide resolved

JevgenijVisockij commented Dec 17, 2020

View reviewed changes

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/StockType.php Show resolved Hide resolved

JevgenijVisockij commented Dec 17, 2020

View reviewed changes

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/StockType.php Show resolved Hide resolved

Julievrz reviewed Dec 18, 2020

View reviewed changes

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/StockType.php Outdated Show resolved Hide resolved

matks added the migration symfony migration project label Dec 18, 2020

matks mentioned this pull request Dec 18, 2020

[EPIC] Symfony Forms simplification using PrestaShop Form Theme #16482

Open

matks previously approved these changes Dec 18, 2020

View reviewed changes

JevgenijVisockij dismissed matks’s stale review via e275499 December 22, 2020 14:14

JevgenijVisockij commented Dec 22, 2020

View reviewed changes

install-dev/data/xml/configuration.xml Show resolved Hide resolved

Julievrz reviewed Dec 23, 2020

View reviewed changes

src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/ProductPreferences/GeneralType.php Outdated Show resolved Hide resolved

Julievrz reviewed Dec 23, 2020

View reviewed changes

Julievrz added the Waiting for author Status: action required, waiting for author feedback label Dec 23, 2020

atomiix previously approved these changes Jan 4, 2021

View reviewed changes

JevgenijVisockij dismissed atomiix’s stale review via 5ed3a5d January 5, 2021 09:03

Julievrz added Wording ✔️ Status: check done, wording approved and removed Waiting for wording Status: action required, waiting for wording labels Jan 5, 2021

Progi1984 reviewed Jan 5, 2021

View reviewed changes

JevgenijVisockij and others added 10 commits March 13, 2021 11:16

Added dot to help message

92d9fae

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

089324d

…ctPreferences/GeneralType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

e4804a1

…ctPreferences/GeneralType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

2740ec6

…ctPreferences/GeneralType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

07d4f54

…ctPreferences/GeneralType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

337bc60

…ctPreferences/PageType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

fdd8421

…ctPreferences/PageType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

f5efa68

…ctPreferences/PaginationType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Update src/PrestaShopBundle/Form/Admin/Configure/ShopParameters/Produ…

462459d

…ctPreferences/PaginationType.php Removed number from error message Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Made product preferences use translate component

5cfa3d1

JevgenijVisockij dismissed stale reviews from matks, atomiix, and Progi1984 via 5cfa3d1 March 13, 2021 09:16

JevgenijVisockij force-pushed the feature/simplify-product-preferences branch from 5ed3a5d to 5cfa3d1 Compare March 13, 2021 09:16

prestonBot added Waiting for wording Status: action required, waiting for wording and removed Wording ✔️ Status: check done, wording approved labels Mar 13, 2021

JevgenijVisockij added 2 commits March 14, 2021 09:42

Restored product controller back to original

7fcd19d

Changed the way errors are passed to controller and handled there

4d86751

Julievrz reviewed Mar 16, 2021

View reviewed changes

src/PrestaShopBundle/Controller/Admin/Configure/ShopParameters/ProductPreferencesController.php Outdated Show resolved Hide resolved

Update src/PrestaShopBundle/Controller/Admin/Configure/ShopParameters…

295a705

…/ProductPreferencesController.php Add then to a translation Co-authored-by: Julie Varisellaz <70583503+Julievrz@users.noreply.github.com>

Julievrz added Wording ✔️ Status: check done, wording approved and removed Waiting for wording Status: action required, waiting for wording labels Mar 31, 2021

Progi1984 reviewed Mar 31, 2021

View reviewed changes

Progi1984 approved these changes Mar 31, 2021

View reviewed changes

micka-fdz mentioned this pull request Sep 10, 2021

Make Shop Parameters > Product Settings forms multistore compatible #25239

Open

PierreRambaud closed this Nov 15, 2021

JevgenijVisockij mentioned this pull request Jun 28, 2023

Improved structure for error message generation in configuration pages #29494

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Simplify product preferences #22471

[WIP] Simplify product preferences #22471

JevgenijVisockij commented Dec 17, 2020

prestonBot commented Dec 17, 2020 •

edited

JevgenijVisockij commented Dec 17, 2020

Julievrz left a comment

matks left a comment

Julievrz commented Dec 21, 2020

JevgenijVisockij commented Dec 22, 2020 •

edited

Julievrz commented Dec 22, 2020

Julievrz left a comment

JevgenijVisockij commented Dec 31, 2020

Progi1984 left a comment

Progi1984 Mar 31, 2021

Progi1984 commented Mar 31, 2021

PierreRambaud commented Oct 5, 2021

PierreRambaud commented Nov 15, 2021

[WIP] Simplify product preferences #22471

[WIP] Simplify product preferences #22471

Conversation

JevgenijVisockij commented Dec 17, 2020

prestonBot commented Dec 17, 2020 • edited

JevgenijVisockij commented Dec 17, 2020

Julievrz left a comment

Choose a reason for hiding this comment

matks left a comment

Choose a reason for hiding this comment

Julievrz commented Dec 21, 2020

JevgenijVisockij commented Dec 22, 2020 • edited

Julievrz commented Dec 22, 2020

Julievrz left a comment

Choose a reason for hiding this comment

JevgenijVisockij commented Dec 31, 2020

Progi1984 left a comment

Choose a reason for hiding this comment

Progi1984 Mar 31, 2021

Choose a reason for hiding this comment

Progi1984 commented Mar 31, 2021

PierreRambaud commented Oct 5, 2021

PierreRambaud commented Nov 15, 2021

prestonBot commented Dec 17, 2020 •

edited

JevgenijVisockij commented Dec 22, 2020 •

edited