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: 5817 Product page: tab Data #5953

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/models/product_raw_data_category.dart';
import 'package:smooth_app/resources/app_icons.dart' as icons;
import 'package:smooth_app/resources/app_icons.dart';

extension CategoryLabelExt on ProductRawDataCategories {
String toL10nString(AppLocalizations appLocalizations) => switch (this) {
ProductRawDataCategories.labels => appLocalizations.label_refresh,
ProductRawDataCategories.category => appLocalizations.category,
ProductRawDataCategories.ingredients => appLocalizations.ingredients,
ProductRawDataCategories.countries =>
appLocalizations.country_chooser_label_from_settings,
ProductRawDataCategories.nutriment => appLocalizations.nutrition,
ProductRawDataCategories.packaging =>
appLocalizations.packaging_information,
ProductRawDataCategories.stores =>
appLocalizations.edit_product_form_item_stores_title,
};

AppIcon toAppIcon() => switch (this) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just in case, return the more generic Widget here

ProductRawDataCategories.labels => const icons.Labels(),
ProductRawDataCategories.category => const icons.Categories(),
ProductRawDataCategories.ingredients => const icons.Ingredients(),
ProductRawDataCategories.countries => const icons.Countries(),
ProductRawDataCategories.nutriment => const icons.NutritionFacts(),
ProductRawDataCategories.packaging => const icons.Packaging(),
ProductRawDataCategories.stores => const icons.Stores(),
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import 'package:smooth_app/pages/product/product_page/raw_data/models/raw_data_element.dart';

class ProductRawDataCategory {
const ProductRawDataCategory(this.category, this.rawDatas);

final ProductRawDataCategories category;
final List<ProductRawDataSubCategory> rawDatas;
}

enum ProductRawDataCategories {
labels,
category,
ingredients,
nutriment,
packaging,
stores,
countries
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
sealed class ProductRawDataSubCategory {}

class ProductRawDataElement extends ProductRawDataSubCategory {
ProductRawDataElement(this.name);

final String name;
}

class ProductRawDataElementDoubleText extends ProductRawDataSubCategory {
ProductRawDataElementDoubleText(this.text1, this.text2);

final String text1;
final String text2;
}

class ProductRawDataSeeMoreButton extends ProductRawDataSubCategory {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import 'package:flutter/material.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/models/raw_data_element.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/product_raw_data_element_item.dart';
import 'package:smooth_app/themes/theme_provider.dart';

class CategoryElementsListView extends StatefulWidget {
const CategoryElementsListView({required this.elements, this.controller});

final List<ProductRawDataSubCategory> elements;
final ScrollController? controller;

@override
State<StatefulWidget> createState() => _CategoryElementsListViewState();
}

class _CategoryElementsListViewState extends State<CategoryElementsListView> {
bool extended = false;

void extendList() {
setState(() {
extended = true;
});
}

@override
Widget build(BuildContext context) {
final List<ProductRawDataSubCategory> listToShow;
if (extended) {
listToShow = widget.elements;
} else {
listToShow = _shortenIfTooLong(widget.elements);
}
final Color dividerColor = context.lightTheme()
? const Color.fromRGBO(228, 228, 228, 1.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two issues here: please use the Color(0xFFxxxxx) syntax instead.
This color is used multiple times in your code.
In that case, please add it to the Extension

Copy link
Author

Choose a reason for hiding this comment

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

You mean add it to the color_extension.dart ? The extension does not seems to be used for personnal color identity.

Copy link
Author

Choose a reason for hiding this comment

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

Ou bien cette extension : context.extension<SmoothColorsThemeExtension ?

: Colors.white;

return Container(
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you use just 1 element from a Container, please use the correct Widget directly.
Here it's a Padding.

margin: const EdgeInsets.only(left: 90.0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please avoid EdgeInsets and prefer the more generic EdgeInsetsDirectional

child: ListView.separated(
controller: widget.controller,
physics: const ClampingScrollPhysics(),
shrinkWrap: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

ShrinkWrap is your enemy. Why do you use it?

Copy link
Author

Choose a reason for hiding this comment

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

What is wrong with ShrinkWrap ?

In my case, it is needed to not have a crash due to ListView inside the other list view.

padding: const EdgeInsets.symmetric(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here + when you have semi-values like, please switch to a predefined constant (eg: MEDIUM_SPACE)

vertical: 11.5,
),
itemBuilder: (BuildContext context, int index) {
return Container(
margin: const EdgeInsets.only(left: 21),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same EdgeInsets -> EdgeInsetsDirectional

child: ProductRawDataElementItem(
listToShow[index], () => extendList()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's one missing trailing comma

);
},
separatorBuilder: (BuildContext context, _) => Divider(
color: dividerColor,
),
itemCount: listToShow.length,
),
);
}

static const int _SUB_LIST_LENGTH = 3;

List<ProductRawDataSubCategory> _shortenIfTooLong(
List<ProductRawDataSubCategory> list) {
if (list.length > _SUB_LIST_LENGTH) {
final List<ProductRawDataSubCategory> toReturn =
Copy link
Collaborator

Choose a reason for hiding this comment

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

A more generic way is to use the following syntax:

[
...list.sublist(0, _SUB_LIST_LENGTH),
ProductRawDataSeeMoreButton(),
]

<ProductRawDataSubCategory>[];
toReturn.addAll(list.sublist(0, _SUB_LIST_LENGTH));
toReturn.add(ProductRawDataSeeMoreButton());
return toReturn;
} else {
return list;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/category_label_ext.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/models/product_raw_data_category.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/product_raw_data_category_elements_item.dart';
import 'package:smooth_app/resources/app_icons.dart' as icons;
import 'package:smooth_app/resources/app_icons.dart';
import 'package:smooth_app/themes/smooth_theme.dart';
import 'package:smooth_app/themes/smooth_theme_colors.dart';
import 'package:smooth_app/themes/theme_provider.dart';

class ProductRawDataCategoryItem extends StatelessWidget {
const ProductRawDataCategoryItem(this.category, {this.controller});

final ProductRawDataCategory category;
final ScrollController? controller;

@override
Widget build(BuildContext context) {
final AppLocalizations appLocalizations = AppLocalizations.of(context);

return Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
_ProductRawDataCategoryTile(category.category.toAppIcon(),
category.category.toL10nString(appLocalizations)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing comma

CategoryElementsListView(
elements: category.rawDatas, controller: controller),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing comma

]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing comma

}
}

class _ProductRawDataCategoryTile extends StatelessWidget {
const _ProductRawDataCategoryTile(this.icon, this.label);

final AppIcon icon;
final String label;
@override
Widget build(BuildContext context) {
final Color contentColor = context.lightTheme()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please save the result of context.lightTheme(). It required some work to get the info

? context.extension<SmoothColorsThemeExtension>().primaryBlack
: Colors.white;

final Color dividerColor = context.lightTheme()
? const Color.fromRGBO(228, 228, 228, 1.0)
: Colors.white;

return Container(
color: const Color.fromRGBO(249, 249, 249, 1.0),
child: Column(
children: <Widget>[
Container(
margin: const EdgeInsetsDirectional.symmetric(vertical: 14),
Copy link
Collaborator

Choose a reason for hiding this comment

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

By convention, when we have double, we use 14.0

//This rows of rows is here to have this Layout Spaced through the lign
child: Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: <Widget>[
//Element icon + label
Row(children: <Widget>[
const SizedBox(width: 31),
Copy link
Collaborator

Choose a reason for hiding this comment

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

By convention, when we have double, we use 31.0

IconTheme(
data: IconThemeData(
color: contentColor,
size: 18.0,
),
child: icon,
),
const SizedBox(width: 14),
Copy link
Collaborator

Choose a reason for hiding this comment

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

By convention, when we have double, we use 14.0

Text(label),
]),
//Edit button
const Row(children: <Widget>[
IconTheme(
data: IconThemeData(
color: Colors.grey,
size: 18.0,
),
child: icons.Edit()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing comma

SizedBox(width: 28),
]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing comma

],
),
),
Divider(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need a Divider of 0?

Copy link
Author

Choose a reason for hiding this comment

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

the divider default value is not 0 but 16

color: dividerColor,
height: 0,
)
],
),
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import 'package:flutter/widgets.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/models/raw_data_element.dart';

class ProductRawDataElementItem extends StatelessWidget {
const ProductRawDataElementItem(this.element, this.onSeeMoreTap,
{this.controller});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing trailing comma


final ProductRawDataSubCategory element;
final ScrollController? controller;
final Function() onSeeMoreTap;

@override
Widget build(BuildContext context) {
switch (element.runtimeType) {
case const (ProductRawDataElement):
return Text((element as ProductRawDataElement).name);
case const (ProductRawDataElementDoubleText):
return Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: <Widget>[
Text((element as ProductRawDataElementDoubleText).text1),
Row(
children: [
Text((element as ProductRawDataElementDoubleText).text2),
const SizedBox(
width: 29,
Copy link
Collaborator

Choose a reason for hiding this comment

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

By convention, when we have double, we use 29.0

)
],
)
],
);
case const (ProductRawDataSeeMoreButton):
{
final AppLocalizations appLocalizations =
AppLocalizations.of(context);
return GestureDetector(
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not fans of GestureDetector. Could you use an InkWell instead?

onTap: () => onSeeMoreTap(),
child: Text(appLocalizations.tap_for_more));
}
default:
throw FormatException('Invalid class ${element.runtimeType}');
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import 'package:flutter/material.dart';
import 'package:openfoodfacts/openfoodfacts.dart';
import 'package:smooth_app/pages/product/attribute_first_row_helper.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/models/product_raw_data_category.dart';
import 'package:smooth_app/pages/product/product_page/raw_data/models/raw_data_element.dart';
import 'package:smooth_app/query/product_query.dart';

extension RawDataExt on Product {
List<ProductRawDataCategory> toRawDatas() {
final List<ProductRawDataCategory> toReturn = <ProductRawDataCategory>[];

final OpenFoodFactsLanguage language = _getLanguage();

_addRawDataInList(toReturn, ProductRawDataCategories.labels,
labelsTagsInLanguages?[language]);

_addRawDataInList(toReturn, ProductRawDataCategories.category,
categoriesTagsInLanguages?[language]);

_addRawDataInList(toReturn, ProductRawDataCategories.ingredients,
_splitString(ingredientsTextInLanguages?[language]));

// TODO(micheldr): Change presentation into two Textfields instead of concatenation.
_addRawDataDoubleTextInList(toReturn, ProductRawDataCategories.nutriment,
AttributeFirstRowNutritionHelper(product: this).getAllTerms());

_addRawDataInList(
toReturn, ProductRawDataCategories.packaging, _splitString(packaging));

_addRawDataInList(
toReturn, ProductRawDataCategories.stores, _splitString(stores));

_addRawDataInList(toReturn, ProductRawDataCategories.countries,
countriesTagsInLanguages?[language]);

return toReturn;
}

void _addRawDataInList(List<ProductRawDataCategory> toBeFilled,
ProductRawDataCategories label, List<String>? toAdd) {
if (toAdd != null) {
toBeFilled.add(ProductRawDataCategory(label, _toRawData(toAdd)));
}
}

void _addRawDataDoubleTextInList(List<ProductRawDataCategory> toBeFilled,
ProductRawDataCategories label, List<StringPair>? toAdd) {
if (toAdd != null) {
toBeFilled
.add(ProductRawDataCategory(label, _toRawDataDoubleText(toAdd)));
}
}

List<ProductRawDataElement> _toRawData(List<String> list) =>
list.map((String element) => ProductRawDataElement(element)).toList();

List<ProductRawDataElementDoubleText> _toRawDataDoubleText(
List<StringPair> list) =>
list
.map((StringPair element) => ProductRawDataElementDoubleText(
element.first, element.second ?? ''))
.toList();

List<String>? _splitString(String? input) {
if (input == null) {
return null;
}
input = input.trim();
if (input.isEmpty) {
return null;
}
return input.split(',');
}

@protected
OpenFoodFactsLanguage _getLanguage() => ProductQuery.getLanguage();
}
Loading
Loading