Skip to content

Conversation

cornifex
Copy link
Contributor

@cornifex cornifex commented Nov 23, 2016

This is just a rough template using all twig with no preprocessing. Definitely needs more work but I've been given other priorities for the time being.

'commerce_order_edit_form' => [
'render element' => 'form',
],
'commerce_order__default' => [
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed w/ @cornifex we'd make an admin view mode and use that instead

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we'd have to alter the default canonical route, Core provides

'_entity_view' => "{$entity_type_id}.full",

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed w/ Bojan. Doesn't really give us a gain. And tests showed all kinds of madness by doing this.

@mglaman mglaman force-pushed the 2757283-template-admin-order-view branch from ce6e9f5 to 5ba9504 Compare November 24, 2016 22:34
{% endfor %}
</div>
</details>
{% for key,value in {'billing_information': 'Billing information'|t, 'shipping_information': 'Shipping information'|t} %}
Copy link
Contributor

Choose a reason for hiding this comment

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

@bojanz I followed the same concepts from the email, since this is pretty hardcoded-ish.

@mglaman
Copy link
Contributor

mglaman commented Nov 25, 2016

@bojanz this is ready to take a peak at.

</div>
<details open>
<summary role="button">
Customer Information
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing |t for Customer information.

#}

{{ attach_library('commerce_order/form') }}
{% set order_state = order_entity.getState.value|capitalize %}
Copy link
Contributor

Choose a reason for hiding this comment

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

You're capitalizing the machine name of the state, instead of taking the label. That's not right.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah so we need getState.getLabel?

}

if ($order->getBillingProfile()) {
$variables['order']['billing_information'] = $order->getBillingProfile()->get('address')->view([
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we not render the entire billing profile? Doing it this way means no phone field / VAT number is shown even if the fields are attached.

Copy link
Contributor

Choose a reason for hiding this comment

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

That works, get can just render it full

{% endfor %}
</div>
</details>
{% for key,value in {'billing_information': 'Billing information'|t, 'shipping_information': 'Shipping information'|t} %}
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like it would be better for TX (themer experience) if these for loops were unrolled.
We have two elements here, repeating their HTML twice is probably no big deal?

Copy link
Contributor

Choose a reason for hiding this comment

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

Works for me

{#
/**
* @file
* Default order template.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add something like "Used on the admin order page." so that the themers know which template they need.

@mglaman mglaman force-pushed the 2757283-template-admin-order-view branch 2 times, most recently from b08663c to 656d4eb Compare November 29, 2016 03:55
@mglaman mglaman force-pushed the 2757283-template-admin-order-view branch from 656d4eb to 8397f4f Compare November 29, 2016 03:56
@mglaman
Copy link
Contributor

mglaman commented Nov 29, 2016

Captain @bojanz, your needs have been addressed

@bojanz
Copy link
Contributor

bojanz commented Nov 29, 2016

Continued in #574.

@bojanz bojanz closed this Nov 29, 2016
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.

3 participants