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.

CommentFileSizeAuthor
#12 2729579-12-field-ui.patch973 bytesmtift
#11 2729579-11-field-ui.patch963 bytesmtift
#8 2729579-8-field-ui.patch1.27 KBmtift
#6 2729579-6-field-ui.patch1.27 KBmtift
#6 interdiff-2729579-6.txt1.15 KBmtift
#4 2729579-field-ui.patch1.28 KBmtift

Comments

bvoran created an issue. See original summary.

markdorison’s picture

markdorison
he/him
commented

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

markdorison
he/him
commented
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

mtift
he/him
Primary language English
Location Minnesota, USA
commented
Status: Active » Needs review
StatusFileSize
new 2729579-field-ui.patch1.28 KB

Good catch. Perhaps we could just do this?

rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented 
+++ 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

mtift
he/him
Primary language English
Location Minnesota, USA
commented
StatusFileSize
new interdiff-2729579-6.txt1.15 KB
new 2729579-6-field-ui.patch1.27 KB

Good catches

rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented
Status: Needs review » Reviewed & tested by the community

Looks good.

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
commented
StatusFileSize
new 2729579-8-field-ui.patch1.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

mtift
he/him
Primary language English
Location Minnesota, USA
commented
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

mtift
he/him
Primary language English
Location Minnesota, USA
commented
Assigned: mtift » Unassigned
Status: Patch (to be ported) » Needs review
StatusFileSize
new 2729579-11-field-ui.patch963 bytes

Here's a Drupal 7 version

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
commented
StatusFileSize
new 2729579-12-field-ui.patch973 bytes

Let's try that again.

rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented
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

mtift
he/him
Primary language English
Location Minnesota, USA
commented
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.