Skip to content

Conversation

@3mpee3mpee
Copy link
Contributor

Changes Made

Currently we don't allow avif images to be uploaded, even if nothing speaks against it since all mayor browsers support it.

Done:

  • check if this works on the flutter app, since flutter itself doesn't support it and relies on the underlying OS. If not, gracefully handle this and display an error message to the user

  • update any i18n strings in the react or flutter app

Related Issue(s)

#2082

Demo Screenshots

image image

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Set a 100 character limit in your editor/IDE to avoid white space diffs in the PR
    (run dart format .)
  • Updated/added relevant documentation (doc comments with ///).
  • Added relevant reviewers.

child: Text(i18n.galleryAvifNotSupported),
),
)
: FadeInImage(
Copy link
Member

Choose a reason for hiding this comment

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

sorry, I guess it wasn't very clear. If the OS supports avif, this will work just fine (e.g. in newer androids, no idea about ios or flathub), we just can't be sure.

we could do something like this

imageErrorBuilder: (BuildContext context, Object error, StackTrace? stackTrace) {
  return Container(
    color: theme.colorScheme.errorContainer,
    child: ...  //show some general error message (add a Icons.broken_image somewhere?)
  );
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I guess I misunderstood, thanks for the clarifications!
currently fixed

},
"gallery": "Galeria",
"@gallery": {},
"galleryAvifNotSupported": "AVIF no és compatible",
Copy link
Member

Choose a reason for hiding this comment

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

and you even added translations! ❤️

(small nitpick, since this will apply to the exercises as well, call the translation key something more general like "imageFormatNotSupported" or so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's makes more sense, agreed.
Updated translations for more generic message as well as the naming

@3mpee3mpee
Copy link
Contributor Author

@rolandgeider please review new changes. Right now how it looks like:
image

spacing: 8,
children: [
Icon(Icons.broken_image),
Text(i18n.imageFormatNotSupportedDetail(imageFormat))
Copy link
Member

Choose a reason for hiding this comment

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

it even displays the type dynamically, nice!

@rolandgeider
Copy link
Member

🚀

@rolandgeider rolandgeider merged commit df37507 into wger-project:master Oct 12, 2025
1 check 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.

2 participants