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 minor issues on Logs Grid page #10281

Merged
merged 5 commits into from Sep 5, 2018

Conversation

Projects
None yet
7 participants
@mickaelandrieu
Contributor

mickaelandrieu commented Sep 4, 2018

Questions Answers
Branch? develop
Description? Fixed some minor issues of Logs page
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixed #10280
How to test? Ask @marionf

This change is Reviewable

@mickaelandrieu mickaelandrieu changed the title from Fix/logs page to Fixed minor issues on Logs Grid page Sep 4, 2018

@mickaelandrieu mickaelandrieu added this to the 1.7.5.0 milestone Sep 4, 2018

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Sep 4, 2018

Member

@mickaelandrieu, your PR already needs a rebase. :)

Member

Quetzacoalt91 commented Sep 4, 2018

@mickaelandrieu, your PR already needs a rebase. :)

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 4, 2018

Contributor

@Quetzacoalt91 I've already rebased it 3 times in 1h lol

Contributor

mickaelandrieu commented Sep 4, 2018

@Quetzacoalt91 I've already rebased it 3 times in 1h lol

@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Sep 4, 2018

Contributor

@mickaelandrieu But you need to run cs-fixer:

Loaded config default from "/home/travis/build/PrestaShop/PrestaShop/.php_cs.dist".
   1) /home/travis/build/PrestaShop/PrestaShop/src/Core/Grid/Definition/Factory/LogGridDefinitionFactory.php
Contributor

PierreRambaud commented Sep 4, 2018

@mickaelandrieu But you need to run cs-fixer:

Loaded config default from "/home/travis/build/PrestaShop/PrestaShop/.php_cs.dist".
   1) /home/travis/build/PrestaShop/PrestaShop/src/Core/Grid/Definition/Factory/LogGridDefinitionFactory.php
@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 4, 2018

Contributor

@mickaelandrieu But you need to run cs-fixer:

something is wrong with our composer command that installs the pre-commit hook

Contributor

mickaelandrieu commented Sep 4, 2018

@mickaelandrieu But you need to run cs-fixer:

something is wrong with our composer command that installs the pre-commit hook

@marionf marionf self-assigned this Sep 5, 2018

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Sep 5, 2018

Contributor

@mickaelandrieu

When I click on erase all and ok, nothing is deleted
And there is a little display issue, the string "Logs by email" shouldn't be displayed above the field

capture d ecran_256

Contributor

marionf commented Sep 5, 2018

@mickaelandrieu

When I click on erase all and ok, nothing is deleted
And there is a little display issue, the string "Logs by email" shouldn't be displayed above the field

capture d ecran_256

@marionf marionf removed their assignment Sep 5, 2018

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 5, 2018

Contributor

When I click on erase all and ok, nothing is deleted

Ok it's now fixed /c @sarjon use DELETE is not compatible with the javascript extension

For the design, I don't know I need help and docs since @eternoendless have changed the form theme from vertical to horizontal view.

Maybe can you hint me on what needs to be done Pablo?

Contributor

mickaelandrieu commented Sep 5, 2018

When I click on erase all and ok, nothing is deleted

Ok it's now fixed /c @sarjon use DELETE is not compatible with the javascript extension

For the design, I don't know I need help and docs since @eternoendless have changed the form theme from vertical to horizontal view.

Maybe can you hint me on what needs to be done Pablo?

@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Sep 5, 2018

Contributor

use DELETE is not compatible with the javascript extension

Since when? Which javascript extension?

Contributor

PierreRambaud commented Sep 5, 2018

use DELETE is not compatible with the javascript extension

Since when? Which javascript extension?

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 5, 2018

Member

Since when? Which javascript extension?

he means grid javascript extension, which is responsible for submiting action. :)

Member

sarjon commented Sep 5, 2018

Since when? Which javascript extension?

he means grid javascript extension, which is responsible for submiting action. :)

@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Sep 5, 2018

Contributor

We maybe need to update javascript extension instead of remove DELETE method :/

Contributor

PierreRambaud commented Sep 5, 2018

We maybe need to update javascript extension instead of remove DELETE method :/

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 5, 2018

Member

hi @marionf "Erase all" action and options layout is fixed. 👍

Member

sarjon commented Sep 5, 2018

hi @marionf "Erase all" action and options layout is fixed. 👍

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 5, 2018

Contributor

We maybe need to update javascript extension instead of remove DELETE method :/

New features on the Grid component are not allowed until the release of 1.7.5, this will probably be done when we will check and update all routes.

Contributor

mickaelandrieu commented Sep 5, 2018

We maybe need to update javascript extension instead of remove DELETE method :/

New features on the Grid component are not allowed until the release of 1.7.5, this will probably be done when we will check and update all routes.

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 5, 2018

Contributor

and options layout is fixed.

I did nothing about it: are you sure? 👼

Can I expect an approval then? 🌹

Contributor

mickaelandrieu commented Sep 5, 2018

and options layout is fixed.

I did nothing about it: are you sure? 👼

Can I expect an approval then? 🌹

@marionf marionf added the QA ✔️ label Sep 5, 2018

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 5, 2018

Member

I did nothing about it: are you sure?

i've updated it. 👍

Member

sarjon commented Sep 5, 2018

I did nothing about it: are you sure?

i've updated it. 👍

@Quetzacoalt91 Quetzacoalt91 merged commit 315e176 into PrestaShop:develop Sep 5, 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
@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91
Member

Quetzacoalt91 commented Sep 5, 2018

Thank you @mickaelandrieu & @sarjon

@mickaelandrieu mickaelandrieu deleted the mickaelandrieu:fix/logs-page branch Sep 5, 2018

@matks matks added the migration label Sep 19, 2018

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