Add a tiny bit of state magic in the image.module and hide the Title and Alt groups when the related form items are disabled

I've been experimenting a bit with this and I have found the following.

Both regional settings and field settings use the same function to populate the list of available column groups (translation_entity_field_sync_widget), when the Translatable elements checkboxes are generated (and default values set).

The proposed patch is for translation_entity because it tries to fix this for generic fields. The restriction is the key of the field being the same as the translation checkbox key (or adding "_field" to it).

Added some feedback to the user in the form description.
The checkboxes are now hidden and disabled if the field is not checked as enabled.

Works as expected. However needs some wording ("Disabled field are not shown" -> "Disabled fields are not shown")

Corrected typo.

Updated patch to work on admin/config/regional/content-language.

Browser coverage

This patch has been tested and proven to work under the following browsers.

• Google Chrome: Version 25.0.1364.160 (Mac OSX)
• Safari: Version 6.0.2 (8536.26.17) (Mac OSX)
• Firefox: Version 19.0.2 (Mac OSX)
• Internet Explorer: Version 8 (Windows XP)

Scenario 1

All elements for the image field (article node bundle) are enabled.

Image settings (all available)

Content language settings (all available)

Scenario 2

Title element is unckecked in the image settings page.

Title element is not show.

Disable title element and then save. Navigate to content language settings.

Fixing the patch to allow this feature.

I review it and seen like it's a small but useful feature.

Patch in #15 from killua99 is a reroll of #13 from e0ipso. There are no differences to review.

#15: drupal-hide-title-and-alt-groups-1920876-15.patch queued for re-testing.

Re-wording and additional screenshot.

Updated issue summary.

There was some code that got lost in the re-roll that happened in #15 that caused the bug described in #23.

im about to test this patch

Updated issue summary.

i followed the steps to reproduce.
the patch is better because the title is hidden, but the group of checkboxes is not moved down.

I don't think we can move the checkboxes down after the required checkboxes. This patch attempts to work not only for the image case, but for other cases too.

Is this fact a blocker?

I followed reroll (https://drupal.org/patch/reroll) and made a new patch. I haven't tested it.

After talking with @YesCT in Prague, we decided to position the Translatable elements at the bottom as an special case for the image module.

cleaning up assigness

Updating issue tag. Will have a look at Drupal Camp Mumbai sprint on this weekend.

@sidharthap: did you intentionally remove those tags along with the one you've added Sidhartha or did the tag monster actually survive the d.o D7 upgrade? If not intended, please add them back. Thanx.

I just updated the tag and unsure how the old tags gets deleted.

...adding accidentally removed tags back then (most of them - not sure if "#SprintWeekend" will be added because of #2171455: d.o does not autocomplete tags starting with # hash anymore).

Patch in #32 needs rerolling.

One conflict was auto-merged. Another conflict had a lot of context changes.

I'm going to test manually #41

I've tested #41 and unfortunally text field still available for be translatable if we have disbled it:

Before apply #41:

After apply #41:

Image field settings in both cases:

I'm going to see what's happening.

I've fixed some of the following things from #41:

+<<<<<<< HEAD
 })(jQuery, Drupal, drupalSettings);
+======= 

+>>>>>>> Applying patch from issue 1920876 comment 7892481

After remove this changes from #41, it still not working :-(. The problem is that 'translation_sync_options' setting isn't arriving to the behaviour. The translation_sync_options setting is set at the 'content_translation_field_sync_widget' function but it doesn't arrive to the behaviour. After a day searching why is happening this, I couldn't find why.

I attach a new patch with the fixes from #41.

Remaining tasks

- Find why this isn't working.
- Next manual tester and, make sure the alt field works similarly (it should be hidden in the translatable enable area when it's not enabled as a field for the content type field setting).
- Add tests.

I'm working on this issue

Re-rolled.
I will continue with this issue

I give it up

Working with @javisr on it, it seems that it wasn't checking if the alt and title where enabled or not. Probably it's not the right way to do it, but at least it seems to work, although I'm not sure that it's working the way it should when we save the admin/config/regional/content-language form.

Hi!
To try what is happening with the test I need to undestand very well what are the expected behaviors.
I write here the testcases of the behavior that i undestood but i dont know if it is well.

[A]: Enable Title field
Enable Alt field

[B]: Image
File
Alt
Title

[1] /admin/config/regional/content-language
[2] /admin/structure/types/manage/article/fields/node.article.field_image?destination=admin/structure/types/manage/article/fields/node.article.field_image

As I understood, while [A] should only appear in [2], [B] should appear in both forms.
The behaivor that I should expect is the next one:

By Default:
- go to [2]
- [A] should appear unchecked
- [B] shouldn't appear.
- go to [1]
- [B] should appear without childrens.

[A] is going to be checked in [2]:
- go to [2]
- check [A]
- [B] should appear unchecked.
- go to [1]
- [B] should appear unchecked.

[A] is checked in [2]:
- [B] should appear in [1] & [2]
- Change any of then
- it should be also change in the other one.

Uncheck [A] in [2]:
- [B] shouldn't appear in [1] and [2]

Could someone confirm if this is the expected behavior please :)

PS: Im sorry about my poor English.

My chance,

Is not so magic, cause the object are diff and make it really hard to handle with JS to hide and show up the checkbox, is better and more secure show or hide the checkbox after submit any post.

This are the obj.
http://drupal8.dev/admin/structure/types/manage/article/fields/node.arti...
Drupal\field\Entity\FieldConfig

http://drupal8.dev/admin/config/regional/content-language
Drupal\field\Entity\FieldInstanceConfig

So the settings are a little diff, that's the reason is so hard to handle with JS.

This is a different way to handle this issue, hope help to solve it.

change status...

Done, added the JavaScript magic (not so magic, when you enabled for first time eg: Alt, the translation sync doesn't popup, but when you disable it, it will hide. The reason; could happen the translation sync is not even setup in the content-translation for that field)

So and I did the fix with the double $definition variable.

This is NW from a JS point of view, will do a proper review over the weekend.

It seems that the manual testing here is done.

seems part of the patch was included already. Light changes to the JS, It's consistent with the rest of the file so I won't touch it further.

This issue is getting bigger. Needs review and close the requirement for this one.

This patch sole an Notice PHP and also prevent the check for some translation_sync when is not selected yet.

Looks good to me

@YesCT this patch fix the problem with the checkbox File in content type field configuration [1].
File doesn't have an enable button so, in JS the $target variable is empty. To don't hide the file button I check it to don't apply the hide() function on the element.

[1] /admin/structure/types/manage/article/fields/node.article.field_image?destination=admin/structure/types/manage/article/fields/node.article.field_image

@YesCT I'm working in this issue :)

@YesCT I've been reviewing the last patch and trying to get a general point of view of the current state of the issue. I have doubts about how should work...sorry :(.

I'm going to try to summarize (I also attach a mind map with all I have reviewed and what I've seen) but I can't promise...

There are two moments, one just after activate the content translation module (before save configuration) and other after configure and save the content translation, setting the article image field translate properties.

1. Before save configuration in admin/config/regional/content-language:

If we go to node.article.field_image

Checkboxes

- In Translatable Elements fieldset we have:
Checbox File: unchecked and visible
Checbox Alt: checked and visible
Checkbox Title: checked and hidden
- In the form we also have:
Checkbox Enable Alt field: Checked
Checkbox Enable Title field: Not checked

At this point, I have a question:
If content translation is not configure for Article content type, why is being showed in Translatable Elements fieldset?

In this form, the JS is working as expected:

If check Enalbe title field -->Translatable Elements title checkbox appears
If check Enable alt field --->Translatable Elements alt checkbox appears
If uncheck Enable title field--> Translatable Elements title checkbox is hidden
If uncheck Enable alt field --->Translatable Elements alt checkbox is hidden
In Translatable Elements, if we check File checkbox, Title and Alt are checked automatically.
Note: hidden is not unchecked, the submitted value for hidden checkboxes could be checked or unchecked.

Currently, there is just one relationship between Translate elements and enable checkboxes. That is, if we check one Enable checkbox (title or alt), the related translate checkbox appears. But, this not affect the translate checkbox value, I mean, if the enable title is checked and the Title checkbox in content translate is checked too, if we uncheck the enable title checkbox, the title checkbox will be hidden and in background will be already checked and if we submit the form, the value for this checkbox will be checked. I just wanna be sure this is the expected functionality.

What happend when the form is submitted?

If Translate Elment has Alt checked
If Translate Elment has Title checked
If Translate Elment has Alt and Title checked

In all cases the cotent translation configuration for article is updated.
At this point, I have the same question. If we don't have set the content translation configuration for Article, could we do this from this form?

2. After save configuration in content-language:

In admin/config/regional/content-language form:

Now,we have set the content translation for Article and we have the following configuration for Image field in Article
Image checkbox is checked
Image > Title checkbox is checked
Image > Alt checkbox is checked

What happen when we change the configuration (submit):

The expecte result is that node.article.field_image should be updated with the changes.
If uncheck Image > Title and image > alt ----> node.article.field_image will not show translatable elements
If uncheck Image > alt ----> alt checkbox in Translatable checkbox will be unchecked
If uncheck Image > title ----> Title checkbox in Translatable checbox will be unchecked

JS is working as expected

If Image > File checkbox is checked Title and Alt are checked and disabled
If Image > File checkbox is unchecked Title and Alt are enabled (and are still checked)
I think this is the correct functionality but I wonder if by default, in node.article.field_image, before save content-language configuration for article, the Translatable Elements shouldn't be in the form. This changes could implies check if there is set the article configuration in content-language instead get directly the default configuration.

Patch no longer applies.

Thanks @jhedstrom I'm on it :)

rerolled

@YesCT I'm not sure about how the translation configuration should work.

I'm working in behat test

In case this is useful, while working on another issue I identified some behaviours that may have contributed to the difficulties encountered with this ticket.

1) The javascript on the admin/config/regional/content-language form will check-all/un-check all the fields (except the "file" field) if/when the checkboxes for bundles are toggled... so while they load correctly to begin with, toggling may result in fields not representing their true "translatable" status (see #2492541: Toggling translatability on admin page shouldn't forget settings)
2) Saving the admin/config/regional/content-language form unexpectedly disables translation for all the (hidden) fields of the bundles for which translation is currently disabled. (see #2240139: Inconsistant behavior when enabling Content Translation) This leads to inconsistent/unexpected behaviour.

Reassigning as no progress for 4 months. Patch no longer applies, marking as needing re-roll.

Straight re-roll, so no interdiff.

Clarify issue summary Problem/Motivation, Proposed resolution, Remaining tasks.

Clarify issue summary User interface changes, API changes.

To clarify the accessibility tag, we need to know whether it's necessary to announce to screenreaders when these checkboxes are shown/hidden. @mgifford?

Updating the issue summary to disseminate some of the extra information to where it makes a bit more sense contextually.

@joelpittet pointed out #825436: Create selenium-RC PIFR plugin for full functional testing as a possible blocker in IRC. Thanks Joël!

Clean up / compact steps to reproduce.

Great, the tests pass. But there are still some things to be done (see issue summary). Back to needs work, I'll see what I can do... I've been wanting to try my hand at Behat tests for a while, so this should be good practice.

Adding Behat tests to https://github.com/mparker17/drupalorg_node_1920876_tests

In my manual tests the disabled fields are shown, although the text "Disabled fields are not shown. Please first enable the fields you need to make translatable." is now visible.

I just did a quick test by enabling the appropriate language modules and then going to https://dfc31.ply.st/admin/structure/types/manage/article/fields/node.ar...

Going here with the alt/title disabled still showed translation an option -
https://dfc31.ply.st/admin/config/regional/content-language

The former's more important as far as improved usability goes..

Also the image in the issue summary needs to change.

@mparker17 - "To clarify the accessibility tag, we need to know whether it's necessary to announce to screenreaders when these checkboxes are shown/hidden"

I didn't follow up on this, but yes. Certainly when the alt tag & title are enabled it would be important to announce that new material is available on the screen. It's important for discoverability.

Updating issue summary.

If you've got time Matt...

I wrote behat test for the following scenario. I used https://www.drupal.org/project/drupalextension for Drupal integration with behat. Behat Drupal Extension module doesn't seem to work with user role and drush. For this test, I gave anonymous user permissions. Documentation for setting up Behat Drupal Extension can be found - https://behat-drupal-extension.readthedocs.org/en/3.1/localinstall.html.

Go to admin/structure/types/manage/article/fields/node.article.field_image.
There is a checkbox labeled "Enable Alt field". It is checked by default.

Updated version of test using Drupal API driver for the coverage of ....
Go to admin/structure/types/manage/article/fields/node.article.field_image.
There is a checkbox labeled "Enable Alt field". It is checked by default.

Behat test for testing scenario 4,5 and 6 are covered in the attached test.