Our clients and project team felt it would be really useful to include a "Manage selected preset" link when editing JW Player settings. This speeds up the process for allowing maintainers to quickly get to the preset they most likely want to edit.

I'll attached the patch in a minute for review.

CommentFileSizeAuthor
#8 interdiff-2713679-6-8.txt3.44 KBjohnchque
#8 add_managed_selected-2713679-8.patch3.59 KBjohnchque
#6 add_managed_selected-2713679-6.patch1.11 KBjohnchque
#2 jw_player-managed_selected_preset-2713679-2.patch1.04 KBron_s
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ron_s created an issue. See original summary.

ron_s’s picture

ron_s CreditAttribution: ron_s commented
Status: Active » Needs review
FileSize
jw_player-managed_selected_preset-2713679-2.patch1.04 KB

Here is the patch for review... this is a very simple addition. Thanks.

  • Berdir committed 669955b on 7.x-2.x authored by ron_s 
    Issue #2713679 by ron_s: Add managed selected preset option
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Version: 7.x-2.x-dev » 8.x-1.x-dev
Status: Needs review » Patch (to be ported)

This could be a bit confusing if you change the setting, but that seems acceptable and not worth to account for.

Committed.

ron_s’s picture

ron_s CreditAttribution: ron_s commented

Yes, I agree it could be... here are three ideas we considered:

1) Use the word "current" in the link, such as "Manage current preset" or "Manage currently saved preset".

2) Add text after the link that says "(only available when saved)".

3) We've done this before with other modules, and would definitely work... #attach an Ajax callback to the select, and automatically change the link on the fly. :-)

Your thoughts? #3?

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Patch (to be ported) » Needs review
FileSize
add_managed_selected-2713679-6.patch1.11 KB

Added extra code for fitting better d8 version.

Status: Needs review » Needs work

The last submitted patch, 6: add_managed_selected-2713679-6.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
add_managed_selected-2713679-8.patch3.59 KB
interdiff-2713679-6-8.txt3.44 KB

Made some changes, added some extra checks and fixed tests.

  • Berdir committed 5e43f28 on 8.x-1.x authored by yongt9412 
    Issue #2713679 by yongt9412, ron_s: Add managed selected preset option
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed 
+++ b/src/Plugin/Field/FieldFormatter/JwplayerFormatter.php
@@ -112,7 +112,7 @@ class JwplayerFormatter extends FormatterBase {
             '#default_value' => $this->getSetting('preview_image_style') ? $this->getSetting('preview_image_style') : '',
             '#states' => [
               'invisible' => [
-                array(':input[name="fields[field_video][settings_edit_form][settings][preview_image_field]"]' => ['value' => '']),
+                array(':input[name="fields[' . $this->fieldDefinition->getName() . '][settings_edit_form][settings][preview_image_field]"]' => ['value' => '']),
               ],

ah, looks like you have the preview states fix here.

Committed, thanks.

Status: Fixed » Closed (fixed)

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