When attempting to "Enable AMP in Custom Display Settings" on D7, field_ui module must be enabled. This step is currently missing from the instructions.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bvoran created an issue. See original summary.

markdorison’s picture

I am guessing this may be in issue on the 8.x branch as well but I have yet to confirm. In addition to adding documentation, it may be beneficial to display a warning or help text on the AMP configuration page that links to the different content types' 'display settings' configuration.

markdorison’s picture

Title: Field UI must be enabled (D7) » 'Enable AMP' content type links send user to wrong destination when field_ui is disabled
Component: Documentation » Code
mtift’s picture

Status: Active » Needs review
FileSize
1.28 KB

Good catch. Perhaps we could just do this?

RainbowArray’s picture

+++ b/src/Form/AmpSettingsForm.php
@@ -126,11 +126,21 @@ class AmpSettingsForm extends ConfigFormBase {
+        '#markup' => $this->t('(In order to enabled and disable AMP content types in the UI, the Field UI module must be enabled.)'),

Should be "In order to enable and disable..."

Otherwise looks solid.

You could choose to use the short array syntax, [] instead of array(). Fine not to change that in this patch though.

mtift’s picture

FileSize
1.15 KB
1.27 KB

Good catches

RainbowArray’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

mtift’s picture

FileSize
1.27 KB

Weird. That patch didn't apply, and it was for the wrong branch. But I'm going to assume this is one will work.

  • mtift committed 64f726e on 8.x-1.x
    Issue #2729579 by mtift: 'Enable AMP' content type links send user to...
mtift’s picture

Assigned: Unassigned » mtift
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

Committed to 8.x-1.x.

mtift’s picture

Assigned: mtift » Unassigned
Status: Patch (to be ported) » Needs review
FileSize
963 bytes

Here's a Drupal 7 version

mtift’s picture

FileSize
973 bytes

Let's try that again.

RainbowArray’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

  • mtift committed 741f16c on 7.x-1.x
    Issue #2729579 by mtift: 'Enable AMP' content type links send user to...
mtift’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x-1.x

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.