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

fix: Truncated button text #4240 #4677

Merged

Conversation

malik-vishu
Copy link
Contributor

Truncated button text

Text truncation problem resolved on Packaging Components screen

  • Default Font size 18 , minimum font size 10
  • borderRadius in ClipRRect was giving error so I had to comment it

Screenshot

Before After

Fixes bug(s)

Issue

  • Issue: type 'BorderRadius?' can't be assigned to the parameter type 'BorderRadiusGeometry' in smooth_draggable_bottom_sheet.dart

Files changed

  • smooth_draggable_bottom_sheet.dart
  • edit_new_packagings.dart
  • product_cards_helper.dart

@malik-vishu
Copy link
Contributor Author

@monsieurtanuki should I update branch?
image

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Thank you @malik-vishu for your PR.
Please have a look at my comments.
The general idea is that you should have a minimal impact on the app and just focus on the issue - e.g. you should not dismiss or change a generic parameter just because it's helpful in that particular case.

@@ -74,7 +74,7 @@ class SmoothDraggableBottomSheetState
child: Material(
type: MaterialType.transparency,
child: ClipRRect(
borderRadius: widget.borderRadius,
// borderRadius: widget.borderRadius,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put it back. If it's a parameter we introduced, let's assume it is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

type 'BorderRadius?' can't be assigned to the parameter type 'BorderRadiusGeometry'
What to do with this error then?

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, as @g123k recently switched to flutter 3.13 (from 3.10), I guess it would make sense to "Update branch". That would probably include the fix to this error.
For the record: when there's an error, the trick is not to comment it, especially if your code gets reviewed 😉

packages/smooth_app/lib/helpers/product_cards_helper.dart Outdated Show resolved Hide resolved
@monsieurtanuki
Copy link
Contributor

@monsieurtanuki should I update branch?

Given that the PR is located in very specific files that are not really connected to major features of the app and with no collision with other PRs, perhaps it's slightly better not to (additional automatic processes and emails and so on). But if you feel like doing it, just do it.

@malik-vishu
Copy link
Contributor Author

@monsieurtanuki Now check.

@malik-vishu
Copy link
Contributor Author

@monsieurtanuki Please check

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Thank you @malik-vishu!

@codecov-commenter
Copy link

Codecov Report

Merging #4677 (39b8d76) into develop (c31e458) will not change coverage.
Report is 1 commits behind head on develop.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop   #4677   +/-   ##
=======================================
  Coverage     9.91%   9.91%           
=======================================
  Files          310     310           
  Lines        15792   15792           
=======================================
  Hits          1566    1566           
  Misses       14226   14226           
Files Coverage Δ
...ric_lib/buttons/smooth_large_button_with_icon.dart 0.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@malik-vishu
Copy link
Contributor Author

@monsieurtanuki Thank you for the chance and direction.

@monsieurtanuki monsieurtanuki merged commit 6820165 into openfoodfacts:develop Oct 2, 2023
6 checks 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.

Truncated button text
3 participants