Images

[MattsAttack]

Description

Implementation of image upload and display for post UI. Also fixes windows build.

Type of Change

• ✨ New feature
• 🐛 Bug fix

Checklist

Before submitting the PR, please make sure you do the following:

• Read the Contributing Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Fill out this template.
• Log your hours.
• Check that commits follow the Angular commit convention, more or less.
• Ideally, include relevant tests that fail without this PR but pass with it (if applicable).

Tested on

• Windows 11

[MattsAttack]

As of this commit, images are uploaded to the Appwrite bucket. Currently working on linking them to the post

[codecov]

Codecov Report

Attention: Patch coverage is 4.67290% with 102 lines in your changes missing coverage. Please review.

Project coverage is 27.84%. Comparing base (129af9a) to head (e0ae8a1).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/app/lib/src/app/wrapper_page.dart	1.85%	53 Missing ⚠️
.../lib/src/features/home/presentation/home/post.dart	0.00%	20 Missing ⚠️
...pp/lib/src/features/home/data/post_repository.dart	9.09%	10 Missing ⚠️
...tures/home/application/uploaded_image_service.dart	0.00%	9 Missing ⚠️
...ib/src/features/home/application/feed_service.dart	0.00%	8 Missing ⚠️
...pp/lib/src/features/auth/data/auth_repository.dart	0.00%	2 Missing ⚠️

Files with missing lines	Coverage Δ
.../app/lib/src/features/home/domain/post_entity.dart	0.00% <ø> (ø)
.../lib/src/features/home/presentation/home/feed.dart	61.11% <ø> (ø)
packages/app/lib/src/utils/api.dart	55.00% <100.00%> (+7.94%)	⬆️
...pp/lib/src/features/auth/data/auth_repository.dart	0.00% <0.00%> (ø)
...ib/src/features/home/application/feed_service.dart	31.03% <0.00%> (-8.10%)	⬇️
...tures/home/application/uploaded_image_service.dart	0.00% <0.00%> (ø)
...pp/lib/src/features/home/data/post_repository.dart	41.30% <9.09%> (-10.13%)	⬇️
.../lib/src/features/home/presentation/home/post.dart	0.00% <0.00%> (ø)
packages/app/lib/src/app/wrapper_page.dart	14.70% <1.85%> (-6.51%)	⬇️

[lishaduck]

Looking pretty good so far! I think the main thing I'm noticing is that .cxx/ needs to get gitignored.

[lishaduck]

@MattsAttack, syncing the IDs prevents multiple images from being attached. Do we want to support multiple images?

[lishaduck]

Just a reminder to gitignore .cxx/*.
You'll need to do an interactive rebase, I'm happy to help next time I see you (do we want to get together for WWT this weekend?).

[MattsAttack]

@lishaduck did you get the code you suggested to work?

[lishaduck]

@lishaduck did you get the code you suggested to work?

I just wrote it in the little box. What diagnostics is build_runner giving?

[MattsAttack]

I just wrote it in the little box. What diagnostics is build_runner giving?

[SEVERE] riverpod_generator on lib/src/features/home/application/uploaded_image_service.dart (cached):

No "build" method found. Classes annotated with @riverpod must define a method named "build".
[SEVERE] Failed after 1.2s

[lishaduck]

You sure you pulled?

[MattsAttack]

You sure you pulled?

Yes. This is what I have right now. The _$UploadedImageService and the build files aren't being generated.

import 'package:fast_immutable_collections/fast_immutable_collections.dart';
import 'package:file_picker/file_picker.dart';
import 'package:flutter/foundation.dart';
import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';

@immutable
@freezed
class UploadedImage {
  // Might need to add IsWeb in here
  UploadedImage({
    required this.imageId, //figure out how to do image ids
    //currently working on multi image upload support. Will need to figure out how to store multiple images. Should just make appwrite imageIds an array of strings and store all ids and loop the upload function
    required this.file,
  });
  String imageId;
  PlatformFile file;
}


@riverpod
class UploadedImageService extends _$UploadedImageService {
  IList<UploadedImage> build() {
    return const IList.empty();
  }

  void addImage(UploadedImage image) {
    state = state.add(image);
  }

  void removeImage(String imageId) {
    // Again, our state is immutable. So we're making a new list instead of
    // changing the existing list.
    state = state.remove(
      state.firstWhere((element) => element.imageId == imageId),
    );
  }
}

[lishaduck]

Oh, add parts. part 'uploaded_image_service.freezed.dart'; part 'uploaded_image_service.g.dart';

[lishaduck]

And another reminder to gitignore .cxx

[MattsAttack]

part 'uploaded_image_service.g.dart';

It compiles now thanks. Is this still a ChangeNotifierProvider or is it something else? Can you send me the docs for this cause this code cause its kind of weird.

Also me and Lucas fixed some of the video quality issues

[lishaduck]

part 'uploaded_image_service.g.dart';

It compiles now thanks. Is this still a ChangeNotifierProvider or is it something else? Can you send me the docs for this cause this code cause its kind of weird.

No. ChangeNotifierProvider is deprecated in favor of NotifierProvider.
This is the way all of the code works.
Docs are at https://riverpod.dev.

Also me and Lucas fixed some of the video quality issues

❤️

[MattsAttack]

No. ChangeNotifierProvider is deprecated in favor of NotifierProvider. This is the way all of the code works.

:(. Not a fan of NotifierProvider

[lishaduck]

No. ChangeNotifierProvider is deprecated in favor of NotifierProvider. This is the way all of the code works.

:(. Not a fan of NotifierProvider

Why? There's basically no differences except that you use state instead of instance vars and you don't need to call notifyListeners

[MattsAttack]

Why? There's basically no differences except that you use state instead of instance vars and you don't need to call notifyListeners

The syntax is strange. I'm also having trouble getting this to work

Also is there a better way to initially watch this?
final uploadedImages = ref.watch(uploadedImagesServiceProvider);
This makes it an IList. I can do a weird cast like this but i feel like theres a better way
final List uploadedImages = ref.watch(uploadedImagesServiceProvider as ProviderListenable<List>);

[lishaduck]

Also is there a better way to initially watch this? final uploadedImages = ref.watch(uploadedImagesServiceProvider); This makes it an IList. I can do a weird cast like this but i feel like theres a better way final List uploadedImages = ref.watch(uploadedImagesServiceProvider as ProviderListenable);

What's the matter with an IList?

[MattsAttack]

@lishaduck if I commit what I have so far would you be willing to fix it? I'm trying to set up the provider in a way so I can implement multi image support. I know how to do it I just don't fully understand how the AsyncNotifer works and I need to go work on other stuff for a bit. If you can fix the provider stuff I should be able implement multi image support tonight

[lishaduck]

@lishaduck if I commit what I have so far would you be willing to fix it? I'm trying to set up the provider in a way so I can implement multi image support. I know how to do it I just don't fully understand how the AsyncNotifer works and I need to go work on other stuff for a bit. If you can fix the provider stuff I should be able implement multi image support tonight

I could, if you could get #145 merged (I'm running 3.29, so I'm stuck right now, but I don't want to merge if it breaks you)
I can Zoom if you'd like, just email me a link.

[MattsAttack]

@lishaduck if I commit what I have so far would you be willing to fix it? I'm trying to set up the provider in a way so I can implement multi image support. I know how to do it I just don't fully understand how the AsyncNotifer works and I need to go work on other stuff for a bit. If you can fix the provider stuff I should be able implement multi image support tonight

I could, if you could get #145 merged (I'm running 3.29, so I'm stuck right now, but I don't want to merge if it breaks you) I can Zoom if you'd like, just email me a link.

I just sent you a link

[lishaduck]

Oh, @MattsAttack, I was reading through the code and realized something: we're using the wrong package. The code won't work on mobile. We should be using image_picker, not file_picker, or else we can't access the camera roll.
I'm happy to work on it, but, as I said earlier, I'd rather we get #145 merged first.

[MattsAttack]

Oh, @MattsAttack, I was reading through the code and realized something: we're using the wrong package. The code won't work on mobile. We should be using image_picker, not file_picker, or else we can't access the camera roll. I'm happy to work on it, but, as I said earlier, I'd rather we get #145 merged first.

Oh boy. I'm almost done with multi image support. I'll let you know when I am and I'll go review #145 if you want to try and tackle image picked. I've looked at the docs for it and it doesn't seem much different

[lishaduck]

Oh boy. I'm almost done with multi image support.

👍

I'll let you know when I am and I'll go review #145

❤️

if you want to try and tackle image picked. I've looked at the docs for it and it doesn't seem much different

✅

[MattsAttack]

@lishaduck have you encountered the issue where the feed scrolls back to the top when new posts are loading? I think it has something to do with the pagination messing with scrolling.

Recording.2025-02-19.111829.mp4

[lishaduck]

@lishaduck have you encountered the issue where the feed scrolls back to the top when new posts are loading? I think it has something to do with the pagination messing with scrolling.

Yeah, I keep hitting it. I tried changing the caching, but I can't catch anything refreshing. I think it can be repro'd on main though, and it only happens if you scroll quickly, so I figure we can just ignore that for now.

[MattsAttack]

Yeah, I keep hitting it. I tried changing the caching, but I can't catch anything refreshing. I think it can be repro'd on main though, and it only happens if you scroll quickly, so I figure we can just ignore that for now.

I keep getting refreshing. But it's fine. Feel free to merge

[lishaduck]

Yeah, I keep hitting it. I tried changing the caching, but I can't catch anything refreshing. I think it can be repro'd on main though, and it only happens if you scroll quickly, so I figure we can just ignore that for now.

I keep getting refreshing. But it's fine. Feel free to merge

Hmmm. Well, we can figure it out later. Are you sure it wasn't there before I messed with stuff?

[MattsAttack]

Hmmm. Well, we can figure it out later. Are you sure it wasn't there before I messed with stuff?

Yeah I've having that issue for a while now. Same with the image refreshing.

[lishaduck]

LGTM!