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

Fix stock design #8326

Merged
merged 19 commits into from Oct 20, 2017

Conversation

Projects
None yet
6 participants
@nihco2
Contributor

nihco2 commented Sep 12, 2017

Questions Answers
Branch? devolop
Description? Fix page display on desktop
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-3939
How to test? Test stock on desktop

This change is Reviewable

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Sep 19, 2017

Member

I think you would be interested in this issue. On firefox, the input fields have their arrows displayed:

capture du 2017-09-19 09-36-29
On Google Chrome, everything seems good.

Member

Quetzacoalt91 commented Sep 19, 2017

I think you would be interested in this issue. On firefox, the input fields have their arrows displayed:

capture du 2017-09-19 09-36-29
On Google Chrome, everything seems good.

@nihco2

This comment has been minimized.

Show comment
Hide comment
@nihco2

nihco2 Sep 21, 2017

Contributor

Thx for this FF test @Quetzacoalt91

Contributor

nihco2 commented Sep 21, 2017

Thx for this FF test @Quetzacoalt91

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Sep 28, 2017

Member

Okay for me!
However, I need you to keep only one commit "BO: compiled assets". It could create conflicts to resolve several times for the other developers, once your PR would be merged.

Member

Quetzacoalt91 commented Sep 28, 2017

Okay for me!
However, I need you to keep only one commit "BO: compiled assets". It could create conflicts to resolve several times for the other developers, once your PR would be merged.

@eternoendless eternoendless added this to the 1.7.3.0 milestone Oct 3, 2017

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Oct 4, 2017

Contributor

Little thing :
On Firefox, the text is stuck against the border
capture du 2017-10-04 18-27-17

On Chrome it's better, there is more place
capture du 2017-10-04 18-29-02

Contributor

marionf commented Oct 4, 2017

Little thing :
On Firefox, the text is stuck against the border
capture du 2017-10-04 18-27-17

On Chrome it's better, there is more place
capture du 2017-10-04 18-29-02

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless

eternoendless Oct 10, 2017

Member

I fixed a lot of small things. It should be OK now!

Member

eternoendless commented Oct 10, 2017

I fixed a lot of small things. It should be OK now!

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Oct 16, 2017

Member

@marionf, could you have a look again please?

Member

Quetzacoalt91 commented Oct 16, 2017

@marionf, could you have a look again please?

@marionf marionf added QA ✔️ and removed waiting for QA labels Oct 16, 2017

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Oct 16, 2017

Contributor

There are still some regression listed here: http://forge.prestashop.com/browse/BOOM-3939
But I don't know if it will be solved on this PR or another ?

Contributor

marionf commented Oct 16, 2017

There are still some regression listed here: http://forge.prestashop.com/browse/BOOM-3939
But I don't know if it will be solved on this PR or another ?

@nihco2

This comment has been minimized.

Show comment
Hide comment
@nihco2

nihco2 Oct 16, 2017

Contributor

@eternoendless to fix the regression, I think here: https://github.com/PrestaShop/PrestaShop/blob/develop/admin-dev/themes/new-theme/js/app/widgets/ps-number.vue#L62
this.value should be a Number instead of a string

Contributor

nihco2 commented Oct 16, 2017

@eternoendless to fix the regression, I think here: https://github.com/PrestaShop/PrestaShop/blob/develop/admin-dev/themes/new-theme/js/app/widgets/ps-number.vue#L62
this.value should be a Number instead of a string

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 18, 2017

Contributor

Reviewed 2 of 8 files at r1, 3 of 3 files at r2, 12 of 12 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


admin-dev/themes/new-theme/js/app/pages/stock/components/header/filters/low-filter.vue, line 35 at r3 (raw file):

      <a class="float-sm-right ml-2" :href="stockImportUrl" target="_blank">
				<span data-toggle="pstooltip" :title="stockImportTitle" data-html="true" data-placement="top">
					<i class="material-icons">cloud_download</i>

Care about tabs instead of spaces


Comments from Reviewable

Contributor

LittleBigDev commented Oct 18, 2017

Reviewed 2 of 8 files at r1, 3 of 3 files at r2, 12 of 12 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


admin-dev/themes/new-theme/js/app/pages/stock/components/header/filters/low-filter.vue, line 35 at r3 (raw file):

      <a class="float-sm-right ml-2" :href="stockImportUrl" target="_blank">
				<span data-toggle="pstooltip" :title="stockImportTitle" data-html="true" data-placement="top">
					<i class="material-icons">cloud_download</i>

Care about tabs instead of spaces


Comments from Reviewable

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless

eternoendless Oct 18, 2017

Member

@nihco2, I can't push into your branch anymore 😢

Could you please check the option "Allow edits from maintainers" on your PR?

You can find it at the bottom of the right column:

image

Thanks!

Member

eternoendless commented Oct 18, 2017

@nihco2, I can't push into your branch anymore 😢

Could you please check the option "Allow edits from maintainers" on your PR?

You can find it at the bottom of the right column:

image

Thanks!

@nihco2

This comment has been minimized.

Show comment
Hide comment
@nihco2

nihco2 Oct 18, 2017

Contributor
Contributor

nihco2 commented Oct 18, 2017

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless

eternoendless Oct 19, 2017

Member

Thanks!

Member

eternoendless commented Oct 19, 2017

Thanks!

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 19, 2017

Contributor

:lgtm:


Reviewed 14 of 14 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

Contributor

LittleBigDev commented Oct 19, 2017

:lgtm:


Reviewed 14 of 14 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless
Member

eternoendless commented Oct 20, 2017

Thank you @nihco2

@eternoendless eternoendless merged commit 99bbff2 into PrestaShop:develop Oct 20, 2017

3 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/reviewable 25 files reviewed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment