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

Allow product ordering on explicit request #11279

Merged
merged 4 commits into from Nov 22, 2018

Conversation

Projects
None yet
6 participants
@jolelievre
Contributor

jolelievre commented Nov 6, 2018

Questions Answers
Branch? 1.7.5.x
Description? The ordering of products was enabled when sort by position, which made the UX confusing. Now ordering is enabled only when the user clicked on the REORDER button, and this mode all filters input are disabled (in consistency with the fact that they are ignored). For more clarity the selected Category name is displayed in the filter select tree.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? #10345
How to test? Go to the Product catalog in BO, perform searches and sorting in normal mode. Select a category, the selected category is now displayed. Click on REORDER button, the filters are disabled and you can frag and drop the items. Order products by position, the drag and drop is disabled, and you can perform some research.

This change is Reviewable

@jolelievre jolelievre force-pushed the jolelievre:search-product branch from 3381c89 to 74d5b8b Nov 6, 2018

@mickaelandrieu

We must not introduce any more business logic in this controller as we plan to remove it at some point: let's extract what can be re-used in the future Product Controller. See PrestaShop/prestafony-project#7 for more information.

@jolelievre

This comment has been minimized.

Contributor

jolelievre commented Nov 12, 2018

@PierreRambaud I got rid of the extract calls
@mickaelandrieu I agree with you, this only allowed to get the selected category without fetching it (since the info was in the Form data), but it was ugly from the start

Instead I fetch the selected category with Category class, which is not super either but at least there is less garbage code
There would be so many ways to perform this better but which would require a lot of refactoring in this controller..

If you still don't like the use of category ObjectModel I will simply remove as it was not really the purpose of this PR/Issue, I just thought showing the selected category made the catalog page much more understandable (for now the info is not shown anywhere and you don't event know you have filtered products from the database filters.. cheap UI experience)

@@ -160,8 +167,7 @@
<input type="button" class="btn btn-outline-secondary" name="products_filter_position_asc" value="{{ "Reorder"|trans({}, 'Admin.Actions') }}" onclick="productOrderPrioritiesTable();" />

This comment has been minimized.

@LouiseBonnard

LouiseBonnard Nov 12, 2018

Contributor

Actually, the wording Reorder is confusing for translators - some get it as ordering something again while it should be understood in the context of changing position... perhaps we should think of a better formulation for future versions? Now it is too late because of string freeze but let's keep it in mind! (w/ @marionf)

This comment has been minimized.

@LouiseBonnard

LouiseBonnard Nov 14, 2018

Contributor

Hey again, the Crowdin community suggested 'Sort order' instead of 'Reorder' to avoid confusion in the back office, what do you think about that?

@marionf

This comment has been minimized.

Contributor

marionf commented Nov 14, 2018

@jolelievre

I have an exception on Catalog page

capture d ecran_620

@marionf marionf removed the waiting for QA label Nov 14, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Nov 21, 2018

@marionf marionf removed their assignment Nov 21, 2018

@PierreRambaud PierreRambaud merged commit f54948b into PrestaShop:1.7.5.x Nov 22, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Contributor

PierreRambaud commented Nov 22, 2018

@PierreRambaud PierreRambaud added this to the 1.7.5.0 milestone Nov 22, 2018

@jolelievre jolelievre deleted the jolelievre:search-product branch Nov 27, 2018

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