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 dropdown buttons space #8616

Merged
merged 1 commit into from Dec 18, 2017

Conversation

Projects
None yet
4 participants
@Quetzacoalt91
Member

Quetzacoalt91 commented Dec 14, 2017

Questions Answers
Branch? 1.7.3.x
Description? With the latest changes in the UI Kit, the drop-down buttons were not displayed properly. This PR fixes the issue.
Type? bug fix
Category? BO
BC breaks? Nope
Deprecations? Nope
Fixed ticket? /
How to test? Go to the module page > Manage tab and check the dropdown buttons

Before

capture du 2017-12-14 15-33-59

After

capture du 2017-12-14 15-35-11


This change is Reviewable

@Quetzacoalt91 Quetzacoalt91 added this to the 1.7.3.0 milestone Dec 14, 2017

@@ -44,12 +44,10 @@
data-confirm_modal="module-modal-confirm-{{ name }}-{{ url_active }}">
{{ url_active|capitalize|replace({'_': " "})|trans({}, 'Admin.Actions') }}
</button>
{% if urls|length > 1 %}
<input type="hidden" class="btn">

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Dec 14, 2017

Contributor

for history, why do we need this hidden button?

@mickaelandrieu

mickaelandrieu Dec 14, 2017

Contributor

for history, why do we need this hidden button?

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Dec 14, 2017

Member

This is the trick. In order to get the bootstrap design with a button near a dropdown, we need two btn side by side.
You can see that without it, I would have a <form> on which I cannot apply the btn class.

@Quetzacoalt91

Quetzacoalt91 Dec 14, 2017

Member

This is the trick. In order to get the bootstrap design with a button near a dropdown, we need two btn side by side.
You can see that without it, I would have a <form> on which I cannot apply the btn class.

This comment has been minimized.

@eternoendless

eternoendless Dec 15, 2017

Member

I still don't understand the reason for the hidden button

@eternoendless

eternoendless Dec 15, 2017

Member

I still don't understand the reason for the hidden button

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Dec 15, 2017

Member

Well, remove it and you will see.

@Quetzacoalt91

Quetzacoalt91 Dec 15, 2017

Member

Well, remove it and you will see.

This comment has been minimized.

@eternoendless

eternoendless Dec 15, 2017

Member

Oh I see now, we have one form per button, and that breaks layout because split buttons need to be side by side.

This needs to be refactored later using formaction so that we have only one form per module.

@eternoendless

eternoendless Dec 15, 2017

Member

Oh I see now, we have one form per button, and that breaks layout because split buttons need to be side by side.

This needs to be refactored later using formaction so that we have only one form per module.

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Dec 15, 2017

Member

We have a form per action per module, because we send them as POST.
We put the maximum code as possible in the template in order to make it work with or without javascript.

@Quetzacoalt91

Quetzacoalt91 Dec 15, 2017

Member

We have a form per action per module, because we send them as POST.
We put the maximum code as possible in the template in order to make it work with or without javascript.

This comment has been minimized.

@eternoendless

eternoendless Dec 18, 2017

Member

formaction should work fine with POST and without javascript. I tried a quick fix but it didn't work. I think the confirmation box was messing something up, but I didn't have time to figure out the source of the problem, so I left it be.

Regarding the need to make it work without javascript... I think it's useless doing that in the BO.

@eternoendless

eternoendless Dec 18, 2017

Member

formaction should work fine with POST and without javascript. I tried a quick fix but it didn't work. I think the confirmation box was messing something up, but I didn't have time to figure out the source of the problem, so I left it be.

Regarding the need to make it work without javascript... I think it's useless doing that in the BO.

@mickaelandrieu mickaelandrieu merged commit d47b888 into PrestaShop:1.7.3.x Dec 18, 2017

2 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Dec 18, 2017

Contributor

Thank you @Quetzacoalt91, and everyone for reviews!

Contributor

mickaelandrieu commented Dec 18, 2017

Thank you @Quetzacoalt91, and everyone for reviews!

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