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

feat(neon_framework): Support blurhashes in NeonImages #2100

Merged
merged 4 commits into from
Jun 6, 2024

Conversation

provokateurin
Copy link
Member

@provokateurin provokateurin commented May 29, 2024

Closes #1583

I could remove the dropping of the non-existent prop here and make a separate PR that we merge right before we make a new breaking release.

Talk doesn't add the blurhash metadata yet for images in chat messages, but I already filed an issue for that: nextcloud/spreed#12351

Testing this on the dev container is not so easy because the metadata isn't populated due to the crons not working as intended. To trigger it manually for a file use ./occ metadata:get --refresh --reset --as-array.

Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 27.77778% with 26 lines in your changes missing coverage. Please review.

Project coverage is 29.97%. Comparing base (fa99616) to head (d449910).
Report is 1 commits behind head on main.

Current head d449910 differs from pull request most recent head 0189240

Please upload reports for the commit 0189240 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2100      +/-   ##
==========================================
- Coverage   30.22%   29.97%   -0.26%     
==========================================
  Files         258      258              
  Lines       84835    84879      +44     
==========================================
- Hits        25640    25441     -199     
- Misses      59195    59438     +243     
Flag Coverage Δ *Carryforward flag
cookie_store 90.51% <ø> (ø) Carriedforward from 6931c4d
dynamite 31.00% <ø> (-0.01%) ⬇️ Carriedforward from 6931c4d
dynamite_end_to_end_test 61.47% <ø> (ø) Carriedforward from 6931c4d
dynamite_runtime 85.40% <ø> (ø) Carriedforward from 6931c4d
neon_dashboard 92.56% <ø> (-3.52%) ⬇️
neon_framework 39.93% <37.50%> (-3.25%) ⬇️
neon_notifications 96.82% <ø> (-3.18%) ⬇️
neon_talk 98.31% <ø> (-1.15%) ⬇️
nextcloud 26.53% <25.00%> (-0.10%) ⬇️
sort_box 90.90% <ø> (ø) Carriedforward from 6931c4d

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
packages/nextcloud/lib/src/webdav/file.dart 100.00% <ø> (ø)
packages/neon_framework/lib/src/widgets/image.dart 74.28% <37.50%> (-4.21%) ⬇️
packages/nextcloud/lib/src/webdav/props.g.dart 25.24% <25.00%> (ø)

... and 50 files with indirect coverage changes

@Leptopoda
Copy link
Member

I have not tested this code yet but looking at the code I'm wondering if we should also show the blurhash when an error occurred?

@provokateurin
Copy link
Member Author

Hm yeah that might make sense, as we at least show something to the user and the user also knows that the image /hasn't/wasn't fully loaded (yet).

Copy link
Member

@Leptopoda Leptopoda left a comment

Choose a reason for hiding this comment

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

LGTM

@provokateurin
Copy link
Member Author

I should probably still change that for errors the blurhash will be shown?

Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
@Leptopoda
Copy link
Member

Yes please

@provokateurin provokateurin merged commit 2b4eaab into main Jun 6, 2024
8 checks passed
@provokateurin provokateurin deleted the feat/neon_framework/blurhash branch June 6, 2024 14:51
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.

Support blurhash
2 participants