Video files using the FileVideoFormatter have a fixed dimension

I just ran into this for a client. I opened an MR, committed #3 as the first commit, and pushed another commit to address most of @alexpott's concerns in #7. Doesn't expand test coverage, so tagging for that.

Saving credit for authors (myself and @zrpnr) and reviewers (@schillerm and @alexpott).

guiu.rocafort.ferrer → made their first commit to this issue’s fork.

I added an additional functional test, checking that the width and height attributes are missing when the formatter settings have a null value in the width and height attributes.

I am removing the Needs tests tag, and changing the status to needs review.

MR appears to have a failure.

The pipeline error seems to be related to this other issue ( https://www.drupal.org/project/drupal/issues/3401988 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active ) and not with the merge request itself. The branch itself is mergeable, though.

Should we still merge the code back to the project ? Or should we make the pipeline run correctly first ?

IMO, this is a really small change, and it also have associated tests, so it should be safe to just accept the merge request.

The second option might require to merge the changes back to the issue branch, so the pipeline is updated with the fix from the issue, and then, push so it runs without hitting the error ? I am not completely sure about this.

Akhil Babu → made their first commit to this issue’s fork.

I have added one more test to validate the HTML outputted by the file formatter when only width or height is set.

Akhil Babu → changed the visibility of the branch 11.x to hidden.

Moving back to needs review

Sadly, there is a failed test,

All tests passed 🎉

Seems close.

Since this is changing the current field formatter believe we will need a CR with screenshots. Screenshots could be added to the issue summary here too.

Hi,
I Have tested the latest MR !5057 on Drupal version 11.0
MR Applied successfully...

Testing steps"

1. Install the Drupal version 11.x
2. Install the Media and Media Library core Modules
3. Go to media types ->video -> manage display ->video types field format -> check for dimension field, it's mandatory
4. Can check on content also, add video media for any content type, and create content by adding different size videos (video size will be fixed as in format)
5. Apply MR, and check for the same

Testing Result:
For the Media type -> video ->dimention field not mandatory
It can save black and videos displayed with actual size. Also able to set fixed dimensions as per requirement

Attaching screenshots and videos as #26

Thanks for testing @Kanchan Bhogade.

I have updated the issue summary and created a Draft change record: https://www.drupal.org/node/3418545 →

Screenshots are added in #27 so removing the tag also I reviewed the CR it seems fine. Leaving it to someone else to review.

CR seems good.

Cleaned up the issue summary as the remaining tasks aren't API changes.

Also screenshots belong in the User interface changes section.

Torn if this is a feature request but will leave as is.

Issue credits

Hiding patch

Left some MR comments

Hi @larowlan, thank you for your suggestions.

I changed the formatter summary text, so there is one line for each width / height attribute that has been set.

I also joined the tests for the different cases, only width, only height, both, and none, in the same test method, changing the field config and loading the page again in the same method.

Waiting to see if all the pipelines run smoothly.

I realized the functional test for testing the multiple width, height, both, none cases was wrong.
Fixed that in last commit.

Feedback appears to be addressed

@smustgrave sorry about that, i will do that in future ocasions. Thanks for pointing that out, i didn't know.

Committed to 11.x and backported to 10.3.x
Published CR

larowlan → closed merge request !5057

Let's also update the config schema to match this change: 📌 Follow-up for #3245446: update `type: field.formatter.settings.file_video` to match Active .

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