Skip to content
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 SyliusTaxationBundle when it's use as a standalone bundle #461

Merged
merged 10 commits into from
Oct 26, 2013

Conversation

BenoitLeveque
Copy link
Contributor

when i was playing with the taxation bundle i've found a bunch of issue, here are the issue i have found:

  • route sylius_tax_category_create and sylius_tax_rate_create are invalid, they contains an id parameters
  • some vars have not a good name such as rates instead of tax_rates and categories instead of tax_categories
  • sorting don't work due to the used of the old syntax _sortable
  • tax rate detail page is empty
  • delete link are just link without the proper http method

<i class="icon-trash"></i> delete
</a>
<form action="{{ path('sylius_tax_category_delete', {'id': tax_category.id}) }}" method="post" class="btn-group inline pull-right">
<input type="hidden" name="_method" value="DELETE" class="btn"><!-- fake sibling to left -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this fake sibling to left comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class and comment are useless i will remove that.

Without this input the button doesn't look like a button which is part of an btn-group

@cordoval
Copy link
Contributor

nice catches! 👶

the input hidden has to be before the button, to make the button part of an btn-group
stloyd added a commit that referenced this pull request Oct 26, 2013
…ndle

Fix SyliusTaxationBundle when it's use as a standalone bundle
@stloyd stloyd merged commit 37497cf into Sylius:master Oct 26, 2013
@stloyd
Copy link
Contributor

stloyd commented Oct 26, 2013

Thanks Benoit! Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants