- ivnish Kazakhstan
I implemented this. Don't work with existing fields. Needs to remove image field from content type and add again
- Status changed to Needs work
about 1 year ago 3:13pm 27 October 2023 - 🇺🇸United States smustgrave
Thanks for working on this old issue!
Will need test coverage and upgrade path for existing fields.
UI changes should have before/after screenshots added to issue summary.
Got the issue summary started but will need updating
- last update
about 1 year ago Patch Failed to Apply - last update
about 1 year ago 30,386 pass, 33 fail - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago Patch Failed to Apply - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 29,576 pass, 31 fail - Merge request !7251Add an Include image in display checkbox to the image field → (Open) created by ivnish
- Assigned to ivnish
- ivnish Kazakhstan
Change record draft created https://www.drupal.org/node/3468525 →
- Issue was unassigned.
- Status changed to Needs review
4 months ago 11:48am 16 August 2024 - 🇮🇳India prashant.c Dharamshala
While reviewing this you need to run database updates by visiting
/update.php
or withdrush updb
Module Update ID Type Description -------- ---------------------- ------------- --------------------------------------------------------- image add_display_checkbox post-update Add a table column for "Enable Display field" checkbox.
otherwise you will get this error
The website encountered an unexpected error. Try again later. Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'field_image_display' in 'where clause': SELECT 1 AS "expression" FROM "node_revision__field_image" "t" WHERE ("field_image_target_id" IS NOT NULL) OR ("field_image_display" IS NOT NULL) OR ("field_image_alt" IS NOT NULL) OR ("field_image_title" IS NOT NULL) OR ("field_image_width" IS NOT NULL) OR ("field_image_height" IS NOT NULL) LIMIT 1 OFFSET 0; Array ( ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->countFieldData() (line 1789 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
The changes are working as expected, now we have the additional checkboxes in the image field settings to display the file.
Steps to test:
- Run database updates using drush or from update.php file
- In the content type or any other entity type add an Image field or edit any existing image field
- In the field settings you will find the checkboxes shown in the attached screenshots
- 🇺🇸United States smustgrave
Update hook does not appear to work.
I ran it and did a drush cex and nothing was updated
Went into admin/structure/media/manage/image/fields and resaved the field (made 0 changes)
And get new config keys.Probably will need to update all image config that ships with core also.
- Status changed to Needs work
4 months ago 2:54pm 17 August 2024 - ivnish Kazakhstan
@smustgrave I tested again and there are results. Needs to view Drupal Status page
1) Before patch there are no errors/warnings about any field
2) After the patch there are errors about fields
3) After updb there are no errors/warnings about any field
Config export still has no changes
- ivnish Kazakhstan
We don't have any changes in configs because Image Storage ALREADY has display property in config (extended from File)
- ivnish Kazakhstan
@smustgrave do we need upgrade path tests if config doesn't change?
- Status changed to Needs review
4 months ago 7:51am 18 August 2024 - 🇺🇸United States smustgrave
I definitely seemed to get config changes after applying the MR. Not from the update hook but from just resaving a field form.
- ivnish Kazakhstan
@smustgrave you are right, but config updated "default image uuid". It is not my patch issue. I installed clean Drupal instance, applied a patch and exported configs. I re-saved the article image field form and exported configs again. Then I compared configs.
See screenshots
- 🇺🇸United States smustgrave
Did you try
Fresh install
Export config
Apply patch
Save field
Export again? - 🇮🇳India prashant.c Dharamshala
@smustgrave in #47 you mean resaving the field by checking the checkbox or without any changes?
- I tested it and on just resaving the image field there are no config changes in my case either.
- By checking the checkbox, it will have a config change as the value is changing to TRUE
display_default: true
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- Status changed to RTBC
14 days ago 9:50pm 10 December 2024 - 🇬🇧United Kingdom longwave UK
It seems this was a deliberate choice at some point, from
ImageItem::isDisplayed()
:// Image items do not have per-item visibility settings.
This was added in #2090619: Move file visibility check to the FileItem class → but there was no discussion that I can find as to why image items should not have this setting.
However, I'm not really sure how useful this feature is. Aren't images meant to be displayed?
- 🇧🇾Belarus krakenbite
However, I'm not really sure how useful this feature is. Aren't images meant to be displayed?
Yes! This is useful feature for some sites. I'm using it on my Drupal 6 projects and want to use on Drupal 10+ after migration
- 🇧🇷Brazil pfeiffer
I created a patch file with the diff of the PR to be more secure to use as a patch
- 🇧🇷Brazil pfeiffer
I tested the PR in a Drupal 11.1.0 and now I have the option for enabling the display control done by each node.
Everything is working and no errors were logged, nice work!
Editing a node:
Editing the field: