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.

Sign up for GitHub

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

Jump to bottom

feat: 3129 Allow to swipe between product images on the full screen image view #3325

Closed
wants to merge 20 commits into from
Closed

feat: 3129 Allow to swipe between product images on the full screen image view #3325

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
512284c
Swipeable view for product images
omkarChend1kar Nov 15, 2022
2772e17
Swipeable view for product images
omkarChend1kar Nov 15, 2022
1288068
Merge branch 'swipeable_product_image_view' of https://github.com/omk…
omkarChend1kar Nov 15, 2022
f4e14cf
Merge branch 'develop' of https://github.com/omkarChend1kar/smooth-ap…
omkarChend1kar Nov 15, 2022
3460fe4
Resolve for formatting issue in file smooth_images_view.dart
omkarChend1kar Nov 16, 2022
5df856b
Resolve for formatting issue in file smooth_images_view.dart
omkarChend1kar Nov 16, 2022
8a3a2f1
Merge branch 'swipeable_product_image_view' of https://github.com/omk…
omkarChend1kar Nov 16, 2022
9866c91
Merge branch 'develop' into swipeable_product_image_view
monsieurtanuki Nov 17, 2022
b589c3c
Merge branch 'develop' of https://github.com/omkarChend1kar/smooth-ap…
omkarChend1kar Nov 18, 2022
2d5136e
1. Refactoring on previous code
omkarChend1kar Nov 18, 2022
693d477
Merge branch 'swipeable_product_image_view' of https://github.com/omk…
omkarChend1kar Nov 18, 2022
5ff8425
Resolved formatting and annotations issue
omkarChend1kar Nov 18, 2022
20fffcb
Code refactoring
omkarChend1kar Nov 21, 2022
739b51d
Merge branch 'develop' into swipeable_product_image_view
omkarChend1kar Nov 26, 2022
9e1ec8b
Formatting changes
omkarChend1kar Nov 26, 2022
bcb3135
Merge branch 'swipeable_product_image_view' of https://github.com/omk…
omkarChend1kar Nov 26, 2022
7b1fbaa
Formatting changes
omkarChend1kar Nov 26, 2022
3bdad42
Merge branch 'swipeable_product_image_view' of https://github.com/omk…
omkarChend1kar Nov 26, 2022
bde4737
feat: 3129 Allow to swipe between product images on the full screen i…
omkarChend1kar Nov 26, 2022
2aa8f9e
feat: 3129 Allow to swipe between product images on the full screen i…
omkarChend1kar Nov 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 26 additions & 21 deletions packages/smooth_app/lib/generic_lib/widgets/images/smooth_images_sliver_grid.dart
View file
Open in desktop
Expand Up @@ -36,28 +36,33 @@ class SmoothImagesSliverGrid extends SmoothImagesView {
padding: const EdgeInsets.all(MEDIUM_SPACE),
sliver: SliverGrid(
delegate: LoadingSliverChildBuilderDelegate(
loading: loading,
childCount: imageList.length,
loadingWidget: _buildShimmer(),
loadingCount: loadingCount,
childBuilder: (BuildContext context, int index) {
final MapEntry<ProductImageData, ImageProvider<Object>?> entry =
imageList[index];
final ImageProvider? imageProvider = entry.value;
final String? imageUrl = entry.key.imageUrl;
loading: loading,
childCount: imageList.length,
loadingWidget: _buildShimmer(),
loadingCount: loadingCount,
childBuilder: (BuildContext context, int index) {
final MapEntry<ProductImageData, ImageProvider<Object>?> entry =
imageList[index];
final ImageProvider? imageProvider = entry.value;
final String? imageUrl = entry.key.imageUrl;

return imageProvider == null || imageUrl == null
? const PictureNotFound()
: Hero(
tag: imageUrl,
child: _ImageTile(
image: imageProvider,
onTap: onTap == null
? null
: () => onTap!(entry.key, entry.value),
),
);
}),
return imageProvider == null || imageUrl == null
? const PictureNotFound()
: Hero(
tag: imageUrl,
child: _ImageTile(
image: imageProvider,
onTap: onTap == null
? null
: () => onTap!(
entry.key,
entry.value,
null,
),
),
);
},
),
gridDelegate: SliverGridDelegateWithMaxCrossAxisExtent(
maxCrossAxisExtent: maxTileWidth,
childAspectRatio: childAspectRatio,
Expand Down
6 changes: 5 additions & 1 deletion packages/smooth_app/lib/generic_lib/widgets/images/smooth_images_sliver_list.dart
View file
Open in desktop
Expand Up @@ -36,7 +36,11 @@ class SmoothImagesSliverList extends SmoothImagesView {
),
onTap: onTap == null
? null
: () => onTap!(imageList[index].key, imageList[index].value),
: () => onTap!(
imageList[index].key,
imageList[index].value,
index,
),
),
),
);
Expand Down
2 changes: 1 addition & 1 deletion packages/smooth_app/lib/generic_lib/widgets/images/smooth_images_view.dart
View file
Open in desktop
Expand Up @@ -10,6 +10,6 @@ abstract class SmoothImagesView extends StatelessWidget {
});

final Map<ProductImageData, ImageProvider?> imagesData;
final void Function(ProductImageData, ImageProvider?)? onTap;
final void Function(ProductImageData, ImageProvider?, int?)? onTap;
final bool loading;
}
33 changes: 24 additions & 9 deletions packages/smooth_app/lib/pages/product/product_image_gallery_view.dart
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@ import 'package:smooth_app/helpers/product_cards_helper.dart';
import 'package:smooth_app/pages/image_crop_page.dart';
import 'package:smooth_app/pages/product/common/product_refresher.dart';
import 'package:smooth_app/pages/product/confirm_and_upload_picture.dart';
import 'package:smooth_app/pages/product/product_image_viewer.dart';
import 'package:smooth_app/pages/product/product_image_swipeable_view.dart';
import 'package:smooth_app/widgets/smooth_app_bar.dart';
import 'package:smooth_app/widgets/smooth_scaffold.dart';

Expand Down Expand Up @@ -101,8 +101,17 @@ class _ProductImageGalleryViewState extends State<ProductImageGalleryView> {
),
SmoothImagesSliverList(
imagesData: _selectedImages,
onTap: (ProductImageData data, _) =>
data.imageUrl != null ? _openImage(data) : _newImage(data),
onTap: (
ProductImageData data,
_,
int? currentProductImageDataIndex,
) =>
data.imageUrl != null
? _openImage(
selectedImages: _selectedImages,
index: currentProductImageDataIndex ?? 0,
)
: _newImage(data),
),
],
),
Expand All @@ -122,14 +131,20 @@ class _ProductImageGalleryViewState extends State<ProductImageGalleryView> {
),
);

Future<void> _openImage(ProductImageData imageData) async =>
Navigator.push<void>(
Future<void> _openImage({
required int index,
required Map<ProductImageData, ImageProvider?> selectedImages,
}) async =>
Navigator.push(
context,
MaterialPageRoute<void>(
builder: (_) => ProductImageViewer(
barcode: _barcode,
imageData: imageData,
),
builder: (_) {
return ProductImageSwipeableView(
barcode: _barcode,
initialProductImageDataIndex: index,
selectedImages: _selectedImages,
);
},
),
);

Expand Down
67 changes: 67 additions & 0 deletions packages/smooth_app/lib/pages/product/product_image_swipeable_view.dart
View file
Open in desktop
@@ -0,0 +1,67 @@
import 'package:flutter/material.dart';
import 'package:smooth_app/data_models/product_image_data.dart';
import 'package:smooth_app/generic_lib/design_constants.dart';
import 'package:smooth_app/generic_lib/widgets/smooth_back_button.dart';
import 'package:smooth_app/pages/product/product_image_viewer.dart';

///Widget to display swipeable product images,
///Opens product image with [initialProductImageDataIndex] from list of images Typecasted from [selectedImages]
///
///Field [selectedImages],[initialProductImageDataIndex],[barcode] cannot be null
omkarChend1kar marked this conversation as resolved.
Show resolved Hide resolved
class ProductImageSwipeableView extends StatelessWidget {
ProductImageSwipeableView({
Key? key,
omkarChend1kar marked this conversation as resolved.
Show resolved Hide resolved
required this.selectedImages,
required this.initialProductImageDataIndex,
required this.barcode,
}) : super(key: key);

final Map<ProductImageData, ImageProvider?> selectedImages;
final int initialProductImageDataIndex;
final String barcode;
final ValueNotifier<int> currentImageDataIndex = ValueNotifier<int>(0);
omkarChend1kar marked this conversation as resolved.
Show resolved Hide resolved

@override
Widget build(BuildContext context) {
WidgetsBinding.instance.addPostFrameCallback((_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure but perhaps would be better in initState.

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, Since I didn't use stateful widget, There was no initstate, And only appBar text that needed to be rebuilt, That's why went with Valuenotifier only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will convert to stateful, That would be more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will convert to stateful, That would be more readable.

Good idea. And you can get rid of ValueNotifier<int>.

currentImageDataIndex.value = initialProductImageDataIndex;
});
final List<MapEntry<ProductImageData, ImageProvider?>> imageList =
selectedImages.entries.toList();
final PageController controller = PageController(
initialPage: initialProductImageDataIndex,
);
return Scaffold(
appBar: AppBar(
backgroundColor: Colors.black,
foregroundColor: WHITE_COLOR,
elevation: 0,
title: ValueListenableBuilder<int>(
valueListenable: currentImageDataIndex,
builder: (_, int index, __) {
return Text(
imageList[index].key.title,
);
},
),
leading: SmoothBackButton(
iconColor: Colors.white,
onPressed: () => Navigator.maybePop(context),
),
),
body: PageView.builder(
onPageChanged: (int index) {
currentImageDataIndex.value = index;
},
controller: controller,
itemCount: selectedImages.keys.length,
itemBuilder: (BuildContext context, int index) {
return ProductImageViewer(
monsieurtanuki marked this conversation as resolved.
Show resolved Hide resolved
barcode: barcode,
imageData: imageList[index].key,
);
},
),
);
}
}
18 changes: 3 additions & 15 deletions packages/smooth_app/lib/pages/product/product_image_viewer.dart
View file
Open in desktop
Expand Up @@ -10,9 +10,7 @@ import 'package:provider/provider.dart';
import 'package:smooth_app/data_models/product_image_data.dart';
import 'package:smooth_app/database/dao_int.dart';
import 'package:smooth_app/database/local_database.dart';
import 'package:smooth_app/generic_lib/design_constants.dart';
import 'package:smooth_app/generic_lib/loading_dialog.dart';
import 'package:smooth_app/generic_lib/widgets/smooth_back_button.dart';
import 'package:smooth_app/helpers/database_helper.dart';
import 'package:smooth_app/pages/product/confirm_and_upload_picture.dart';
import 'package:smooth_app/widgets/smooth_scaffold.dart';
Expand All @@ -37,12 +35,13 @@ class _ProductImageViewerState extends State<ProductImageViewer> {

/// When the image is edited, this is the new image
Copy link
Contributor

Choose a reason for hiding this comment

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

No

late ImageProvider imageProvider;
bool _isEdited = false;

@override
void initState() {
imageData = widget.imageData;
imageProvider = NetworkImage(imageData.imageUrl!);
imageProvider = NetworkImage(
imageData.imageUrl ?? '',
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not possible: if there's nothing to display you shouldn't be there with a non-existing image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh ,Right I should have first check what is behaviour when images are not present, That was presumptuous of me to consider images will always be present.

Copy link
Contributor

Choose a reason for hiding this comment

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

@omkarChend1kar Thinking again about it, it's not that simple either, as the imageUrl can be refreshed from outside (perhaps not yet, but that's part of #3018 that I'm implementing).

That would mean that if on the gallery you click on an existing image, you go to a swipe page with all images, existing or non existing. On a non existing image, you have to display something specific like the "no picture" logo, with a "new image" button: that's what you should change from your PR code.
And if on the gallery you click directly on a non existing image, you go directly to the "new image" feature (like now).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@monsieurtanuki Actually when imageUrl is without proper path or let say empty string, As I have declared there, It shows that No image icon, That's why I was putting empty string there instead of passing null, Since it was throwing nulll exception, After what you said about this, I thought of filtering list with non null Image paths and then show only uploaded images of whichever category would be displayed,

But I guess this is what you have just mentioned above right,To show the icon/ text for non-existing images.

noimagedisplay

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't agree: this is not NetworkImage's job to display crap.
That's our job to display something relevant when we do know there's no image there.
And in the previous screen does display something when there's no image - we have a specific "no image" logo. At least for consistency, we should display the same logo.

And, I think I already mentioned that earlier, this in NOT the same behavior when you click on "edit": either you modify an existing image, or you create a new image. In your case you always try to modify, and that will fail with no source image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although, I do have one query, After I put our specified logo image at the place of non-existing image source, It doesn't make to display edit button, Instead of that what if we put Add button there and do whatever we do in previous image gallery place such that it will user to add image there itself, Still not sure how going to implement it but just that for non-existing image add buttons looks better.

Copy link
Contributor

Choose a reason for hiding this comment

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

@omkarChend1kar That's correct - for the "no image" cases we need:

  • a specific "no image" logo, instead of the image
  • a specific "add" button label, instead of "edit"
  • a specific "add" action, instead of "edit"

);

super.initState();
}
Expand All @@ -60,16 +59,6 @@ class _ProductImageViewerState extends State<ProductImageViewer> {
_editImage(daoInt);
},
),
appBar: AppBar(
backgroundColor: Colors.black,
foregroundColor: WHITE_COLOR,
elevation: 0,
title: Text(imageData.title),
leading: SmoothBackButton(
iconColor: Colors.white,
onPressed: () => Navigator.maybePop(context, _isEdited),
),
),
body: Column(
mainAxisAlignment: MainAxisAlignment.center,
children: <Widget>[
Expand Down Expand Up @@ -122,7 +111,6 @@ class _ProductImageViewerState extends State<ProductImageViewer> {
),
);
if (photoUploaded != null) {
_isEdited = true;
if (!mounted) {
return;
}
Expand Down