Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to get alt text from Original File. #1024

Merged
merged 4 commits into from
Jul 4, 2024

Conversation

rosiel
Copy link
Member

@rosiel rosiel commented May 31, 2024

GitHub Issue: Islandora/documentation#2310

What does this Pull Request do?

When configuring an image formatter (e.g. in Manage Display), you can now choose to get your alt text from the Original File.

This would work well if we made Alt Text not mandatory. When alt text is mandatory, the derivatives come with alt text equal to the filename. (When alt text is not mandatory, the derivatives don't have alt text).

What's new?

Options for alt text:

  • none (dont render alt text; useful for thumbnails or decorative images)

  • Local (get the alt text from the media being displayed)

  • Local with fallback to Original File (use local alt text if present, otherwise get it from the original file)

  • Original file (use the alt text from an original file).

  • Does this change add any new dependencies? no

  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? no

  • Could this change impact execution of existing code? no

How should this be tested?

  • Make alt text non-required.
  • Make some content on your site including images and files (where the derivatives are images), and add alt text to the original files (in the case where the original file is an image)
  • Select one of the various settings for alt text in Image -> Manage Display -> Source [important! Select the Source View Mode] -> field_media_image.
  • View the objects, you should be viewing the alt text from the original file when applicable, and it should behave reasonably in all other situations.

Documentation Status

  • Does this change existing behaviour that's currently documented? yes, ish
  • Does this change require new pages or sections of documentation? maybe
  • Who does this need to be documented for? everyone
  • Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@mjordan
Copy link

mjordan commented Jun 4, 2024

Testing this, and I think I have it configured correctly:

image

But the alt text from my Original File is not propagating down to Service File or Thumbnail. Can confirm this is from admin/structure/media/manage/image/display/source and that I have clicked the "Save" button at the bottom of the field list/settings are sticking. Any suggestions?

@rosiel
Copy link
Member Author

rosiel commented Jun 4, 2024

I had to clear cache a lot. And I see you have it on "source" which is the view mode of the default display - that's better than I did several times while testing. 😅

@rosiel
Copy link
Member Author

rosiel commented Jun 4, 2024

Also I'm not sure what you're doing to look at thumbnails. The current starter site used DGI Image Discovery which doesn't work with this (it has its own display form). And if you turned on the option for media to have their own URLs, they're probably displaying as Default view mode.

So this only works in the starter site with service file images that get displayed on the node.

@mjordan
Copy link

mjordan commented Jun 4, 2024

Sorry, no luck after pulling in your latest changes, resaving my display mode config form, and clearing the cache a bunch of times. I am now getting the following error when I view an image node with service file:

Error: Class "Drupal\islandora\Plugin\Field\FieldFormatter\Orig.IslandoraImageFormatter" not found in Drupal\Core\Field\FormatterPluginManager->getInstance() (line 113 of /var/www/drupal/web/core/lib/Drupal/Core/Field/FormatterPluginManager.php).

@rosiel
Copy link
Member Author

rosiel commented Jun 5, 2024

Orig.IslandoraImageFormatter

I don't understand how this could happen.

@mjordan
Copy link

mjordan commented Jun 5, 2024

I did something stupid. That explains a lot! Let me test again later.

Copy link
Member

@jordandukart jordandukart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't able to reproduce @mjordan's issues, noted a couple things. Can assign back to me when you have a chance to sort through them. Feel free to ping me for clarity in Slack :)

For anyone else looking to test you should be able to do the following:

  1. Create a media view.
  2. Use the image field.
  3. Configure the above field to use the IslandoraImage formatter and change the options as @rosiel describes.

@rosiel
Copy link
Member Author

rosiel commented Jun 12, 2024

Further testing instructions:

  1. Create an Image node, and add an Image media that is Original File. In the Original File, after you upload the image, change the automatic alt text to actual descriptive alt text. Save the media.
  2. Go to the node's page. You should see an image, though it'll be the Service File not the Original File. If you go to the alt text, it will likely be "filename-Service File.jpg" (i.e. the name of the service file media).
  3. The service file on the node's page is being rendered through the Source view mode (this is a Media view mode), so go to Media Types > Image > Manage Display > Source (!!! don't forget this last bit!). the Source view mode should display only the image. Click the gear to edit and you should see the Alt Text Source as an option. If you change it to Original File, then you should see the descriptive alt text you added. If you change it to Local, with fallback to original file, then you will see the local (service file's) alt text (the filename).

If you make the alt text not mandatory (this is a Starter Site configuration) then the service file will not be automatically populated with bad alt text. I suggest we do this, once this is merged.

If we don't populate the alt text with the title, then we will be encouraged to write real alt text, but i'll leave this to a different PR.

Copy link
Member

@seth-shaw-asu seth-shaw-asu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised.

@seth-shaw-asu seth-shaw-asu merged commit 14c4e42 into Islandora:2.x Jul 4, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants