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

introduces moved search and reset type button to the right #11877

Merged
merged 6 commits into from Jan 10, 2019

Conversation

Projects
None yet
6 participants
@tomas862
Copy link
Member

tomas862 commented Dec 20, 2018

Questions Answers
Branch? develop
Description? it moves search and reset buttons to the right on all viewport widths and this change is included to all lists. Problem mentioned here #10774 (comment)
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? just take a look to all migrated lists to see if the button in cases are in the right. As well, check the reset button if its also aligned to the right in all viewports

how it looks now:

button_right

reset_button


This change is Reviewable

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Dec 20, 2018

I think we should move also the "actions" label to the right, what do you think @TristanLDD ?

@TristanLDD

This comment has been minimized.

Copy link

TristanLDD commented Dec 20, 2018

@marionf Yep, the label should be above the search button

@marionf marionf removed the waiting for UX label Dec 20, 2018

@tomas862

This comment has been minimized.

Copy link
Member Author

tomas862 commented Dec 20, 2018

@marionf and @TristanLDD - is it good or the "Actions" should be aligned with the button text itself?
alignment

@tomas862

This comment has been minimized.

Copy link
Member Author

tomas862 commented Jan 4, 2019

@TristanLDD I aligned "Actions" text with the button "Search" text

alignment1
and in another language

alignment2

previously it was as I mentioned in the comment before 🙂

@TristanLDD

This comment has been minimized.

Copy link

TristanLDD commented Jan 7, 2019

Good for me :)

@@ -0,0 +1,3 @@
<div class="float-right grid-actions-header-text">
<span>{{ column.name }}</span>

This comment has been minimized.

@matks

matks Jan 7, 2019

Contributor

🤔 What is the use of this twig template ?

This comment has been minimized.

@tomas862

tomas862 Jan 7, 2019

Author Member

it overrides grids actions header text- without this template, it would be just a plain text with no unique class to identify it.

With that, instead of just always floating the last element in the grid I'm targeting the specific case I need : To float the text only in actions column. And I added some more commits which now floats only then, when the actions column is in the last place of the grid 😃

This comment has been minimized.

@matks

matks Jan 8, 2019

Contributor

I'm wondering ... we currently have a default.html.twig header with a if inside:
https://github.com/PrestaShop/PrestaShop/blob/develop/src/PrestaShopBundle/Resources/views/Admin/Common/Grid/Columns/Header/Content/default.html.twig#L30
and another usecase where we use the name of the template to override:
https://github.com/PrestaShop/PrestaShop/blob/develop/src/PrestaShopBundle/Resources/views/Admin/Common/Grid/Columns/Header/Content/position_handle.html.twig

Should we keep using name overriding or add more if in the default file ?

This comment has been minimized.

@tomas862

tomas862 Jan 8, 2019

Author Member

well , adding an if statement just to add class in twig seems for me not so clean approach - I would rather prefer the template override as it is now, since if someone would like to change the class name or something in that template then that user won't have to look throw if statement to find out whats going on there 🤔

This comment has been minimized.

@matks

matks Jan 9, 2019

Contributor

Fair point 👍

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jan 7, 2019

@tomas862 Got 1 question, else code is 👍

tomas862 added some commits Jan 7, 2019

removes float right class and adds floating in the css only to preven…
…t different css styles when the actions column is in unexpected place
removes float right class and adds floating in the css only to preven…
…t different css styles when the actions column is in unexpected place
@matks

matks approved these changes Jan 9, 2019

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jan 10, 2019

Thank you @tomas862 !

@matks matks merged commit dd82133 into PrestaShop:develop Jan 10, 2019

1 check passed

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

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Jan 10, 2019

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