search

lukepighetti / salomon_bottom_bar

Yet another bottom navigation bar, but with a few key promises.
https://pub.dev/packages/salomon_bottom_bar
MIT License
258 stars 55 forks source link

✅Background Color (Hopefully this passes the minimal diff...) #44

Closed georonathan47 closed 1 year ago

georonathan47 commented 1 year ago

Entire SalomonBottomBar widget is wrapped with the ColoredBox widget, allowing us to use the backgroundColor we prefer.

Hopefully this passes the minimal diff...

georonathan47 commented 1 year ago

Will check and revert

On Sat, Mar 18, 2023 at 8:38 PM Luke Pighetti @.***> wrote:

@.**** requested changes on this pull request.

Please see comments, thank you!

On example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151726 :

remove this diff

On example/ios/Runner/Info.plist https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151754 :

remove this diff

In example/lib/main.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151759 :

@@ -24,10 +24,12 @@ class _MyAppState extends State { visualDensity: VisualDensity.adaptivePlatformDensity, ), home: Scaffold(

  • // backgroundColor: Colors.grey,

remove this diff

On example/pubspec.lock https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151776 :

remove this diff

On pubspec.lock https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151804 :

remove this diff

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151884 :

  • padding: Directionality.of(context) == TextDirection.ltr
  • ? EdgeInsets.only(left: itemPadding.left / 2, right: itemPadding.right)
  • : EdgeInsets.only(left: itemPadding.left, right: itemPadding.right / 2),

format this file to 80 characters as per dartfmt specs

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151899 :

  • child: SizedBox(
  • /// TODO: Constrain item height without a fixed value
  • ///
  • /// The Align property appears to make these full height, would be
  • /// best to find a way to make it respond only to padding.
  • height: 20,
  • child: Align(
  • alignment: Alignment(-0.2, 0.0),
  • widthFactor: t,
  • child: Padding(
  • padding: Directionality.of(context) == TextDirection.ltr
  • ? EdgeInsets.only(left: itemPadding.left / 2, right: itemPadding.right)
  • : EdgeInsets.only(left: itemPadding.left, right: itemPadding.right / 2),
  • child: DefaultTextStyle(
  • style: TextStyle(
  • color: Color.lerp(_selectedColor.withOpacity(0.0), _selectedColor, t),

format line width

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151941 :

  • highlightColor: _selectedColor.withOpacity(0.1),
  • splashColor: _selectedColor.withOpacity(0.1),
  • hoverColor: _selectedColor.withOpacity(0.1),
  • child: Padding(
  • padding: itemPadding -
  • (Directionality.of(context) == TextDirection.ltr
  • ? EdgeInsets.only(right: itemPadding.right * t)
  • : EdgeInsets.only(left: itemPadding.left * t)),
  • child: Row(
  • children: [
  • IconTheme(
  • data: IconThemeData(
  • color: Color.lerp(_unselectedColor, _selectedColor, t),
  • size: 24,
  • ),
  • child: items.indexOf(item) == currentIndex ? item.activeIcon ?? item.icon : item.icon,

format line width

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151956 :

  • onTap: () => onTap?.call(items.indexOf(item)),
  • customBorder: itemShape,
  • focusColor: _selectedColor.withOpacity(0.1),
  • highlightColor: _selectedColor.withOpacity(0.1),
  • splashColor: _selectedColor.withOpacity(0.1),
  • hoverColor: _selectedColor.withOpacity(0.1),
  • child: Padding(
  • padding: itemPadding -
  • (Directionality.of(context) == TextDirection.ltr
  • ? EdgeInsets.only(right: itemPadding.right * t)
  • : EdgeInsets.only(left: itemPadding.left * t)),
  • child: Row(
  • children: [
  • IconTheme(
  • data: IconThemeData(
  • color: Color.lerp(_unselectedColor, _selectedColor, t),

format line width

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151961 :

  • children: [
  • for (final item in items)
  • TweenAnimationBuilder(
  • tween: Tween(
  • end: items.indexOf(item) == currentIndex ? 1.0 : 0.0,
  • ),
  • curve: curve,
  • duration: duration,
  • builder: (context, t, _) {
  • final _selectedColor = item.selectedColor ?? selectedItemColor ?? theme.primaryColor;
  • final _unselectedColor = item.unselectedColor ?? unselectedItemColor ?? theme.iconTheme.color;
  • return Material(
  • color: Color.lerp(
  • _selectedColor.withOpacity(0.0), _selectedColor.withOpacity(selectedColorOpacity ?? 0.1), t),

format line width

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151985 :

  • child: Row(
  • /// Using a different alignment when there are 2 items or less
  • /// so it behaves the same as BottomNavigationBar.
  • mainAxisAlignment: items.length <= 2 ? MainAxisAlignment.spaceEvenly : MainAxisAlignment.spaceBetween,
  • children: [
  • for (final item in items)
  • TweenAnimationBuilder(
  • tween: Tween(
  • end: items.indexOf(item) == currentIndex ? 1.0 : 0.0,
  • ),
  • curve: curve,
  • duration: duration,
  • builder: (context, t, _) {
  • final _selectedColor = item.selectedColor ?? selectedItemColor ?? theme.primaryColor;
  • final _unselectedColor = item.unselectedColor ?? unselectedItemColor ?? theme.iconTheme.color;

format line width

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151987 :

  • child: SafeArea(
  • minimum: margin,
  • child: Row(
  • /// Using a different alignment when there are 2 items or less
  • /// so it behaves the same as BottomNavigationBar.
  • mainAxisAlignment: items.length <= 2 ? MainAxisAlignment.spaceEvenly : MainAxisAlignment.spaceBetween,
  • children: [
  • for (final item in items)
  • TweenAnimationBuilder(
  • tween: Tween(
  • end: items.indexOf(item) == currentIndex ? 1.0 : 0.0,
  • ),
  • curve: curve,
  • duration: duration,
  • builder: (context, t, _) {
  • final _selectedColor = item.selectedColor ?? selectedItemColor ?? theme.primaryColor;

format line width

In lib/salomon_bottom_bar.dart https://github.com/lukepighetti/salomon_bottom_bar/pull/44#discussion_r1141151989 :

  • : EdgeInsets.only(left: itemPadding.left * t)),
  • child: Row(
  • children: [
  • IconTheme(
  • data: IconThemeData(
  • color: Color.lerp(
  • _unselectedColor, _selectedColor, t),
  • size: 24,
  • return ColoredBox(
  • color: backgroundColor ?? theme.bottomAppBarColor,
  • child: SafeArea(
  • minimum: margin,
  • child: Row(
  • /// Using a different alignment when there are 2 items or less
  • /// so it behaves the same as BottomNavigationBar.
  • mainAxisAlignment: items.length <= 2 ? MainAxisAlignment.spaceEvenly : MainAxisAlignment.spaceBetween,

format line width

— Reply to this email directly, view it on GitHub https://github.com/lukepighetti/salomon_bottom_bar/pull/44#pullrequestreview-1347227739, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANRVGIVS3YFENLQQ7WNCSTDW4YMNNANCNFSM6AAAAAAV7LBGOE . You are receiving this because you authored the thread.Message ID: @.***>

georonathan47 commented 1 year ago

Reverted all previously changed lines. Line width is now 80 characters as per dart regulations

georonathan47 commented 1 year ago

Sure🥳🥳🥳I’ll set it to transparentOn 20 Mar 2023, at 15:50, Luke Pighetti @.***> wrote: @lukepighetti requested changes on this pull request.

almost there!

On pubspec.lock: remove this diff

In lib/salomon_bottom_bar.dart:

  • splashColor: _selectedColor.withOpacity(0.1),
  • hoverColor: _selectedColor.withOpacity(0.1),
  • child: Padding(
  • padding: itemPadding -
  • (Directionality.of(context) == TextDirection.ltr
  • ? EdgeInsets.only(right: itemPadding.right * t)
  • : EdgeInsets.only(left: itemPadding.left * t)),
  • child: Row(
  • children: [
  • IconTheme(
  • data: IconThemeData(
  • color: Color.lerp(
  • _unselectedColor, _selectedColor, t),
  • size: 24,
  • return ColoredBox(
  • color: backgroundColor ?? theme.bottomAppBarColor,

⬇️ Suggested change

While theme.bottomAppBarColor is probably the right choice, it is a breaking change. So let's default to Colors.transparent so we don't have to do a major version release

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

georonathan47 commented 1 year ago

⬇️ Suggested change - color: backgroundColor ?? theme.bottomAppBarColor, + color: backgroundColor ?? Colors.transparent

Changes in this PR have been made in order to prevent a major breaking change

lukepighetti commented 1 year ago

resolves #32