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

Refactor top bar and navigation control widgets #31

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

ulusoyca
Copy link
Collaborator

@ulusoyca ulusoyca commented Aug 6, 2023

Summary

This PR significantly changes the API from top bar and navigation bar aspects.

Changes

Option to set top bar always visible

  /// Indicates whether the top bar should always remain visible, regardless of the page scroll
  /// position. When set to true, the top bar will be permanently displayed; when false, it may
  /// be hidden or revealed  based on the page's scrolling behavior.
  final bool isTopBarLayerAlwaysVisible;

Example use:

isTopBarALwaysVisible.mov

Layering on z axis

  • Clearly separates the top bar and navigation bar. On z axis, the top bar resides above the main content and below the transparent navigation bar.

modal_sheet_page

  /// On z axis, the Top Bar resides above the main content and below the transparent navigation
  /// bar.
  ///
  /// Top bar aids users in grasping the context by displaying an optional title. The height of
  /// the top bar is equal to the height of [navigationBarHeight]. In other saying, when visible,
  /// the top bar fills the transparent background of the navigation bar.
  ///
  /// In scenarios where sheets are filled with content requiring scrolling, by default the top
  /// bar becomes visible as the  user scrolls, causing the page title replaced by the top bar.
  /// At this point, the top bar adopts a 'sticky' position at the top, guaranteeing consistent
  /// visibility. When [isTopBarAlwaysVisible] is set to true, the top bar will be permanently
  /// sticky at the top of the sheet.
  ///
  /// The Top Bar design is flexible, when [hasTopBarLayer] is set to false, the top bar and the
  /// [topBarTitle] will be hidden.
  final bool hasTopBarLayer;

  /// Height of the navigation bar. This value will also be the height of the top bar.
  ///
  /// The navigation bar layer has a transparent background, and sits directly above the top bar
  /// on the z-axis. 
  /// 
  /// It includes two specific widgets:
  /// the [leadingNavBarWidget] and the [trailingNavBarWidget]. The leading widget usually
  /// functions as the back button, enabling users to navigate to the previous page. The trailing
  /// widget often serves as the close button, utilized to close the modal sheet. Together, these
  /// widgets provide clear and intuitive navigational control, differentiating themselves from
  /// the top bar by focusing specifically on directional navigation within the interface.
  final double navigationBarHeight;

Optional top bar layer

  • When the WoltModalSheetPage.hasTopBarLayer is set to false, top bar layer is never shown.
no_top_bar.mov

WoltNavigationToolbar

  • Introduces the WoltNavigationToolbar for internal use which accomodates the navigation control widgets as leading and trailing. The top bar title also sits in the middle of WoltNavigationToolbar widget. This widget modifies the Flutter SDK's NavigationToolBar in a way that it ensures a symmetric layout of widgets in the absence of either a leading or trailing widget, which makes for a visually balanced UI.
ex. 1 ex. 2
image image

Leading and trailing navigation control widgets

  • Renames the closeButton and backButton fields of WoltModalSheet as trailingNavBarWidget and leadingNavBarWidget to indicate them more generic use.
image
  /// A widget representing leading widget in the navigation toolbar. This widget is usually
  /// a the back button.
  final Widget? leadingNavBarWidget;

  /// A widget representing trailing widget in the navigation toolbar. This widget is usually
  /// a the close button.
  final Widget? trailingNavBarWidget;

@ulusoyca ulusoyca force-pushed the use-navigation-toolbar-for-navigation-controls branch from d6193fd to 8120fc1 Compare August 6, 2023 20:00
@MuTe33 MuTe33 self-assigned this Aug 7, 2023
Copy link
Collaborator

@TahaTesser TahaTesser left a comment

Choose a reason for hiding this comment

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

LGTM

/// width of the leading widget. Similarly, if there is no trailing widget and
/// there is a leading widget, then the width of the leading widget is used as the
/// width of the trailing widget.
class WoltNavigationToolbar extends StatelessWidget {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ulusoyca
Is this any different than https://github.com/TahaTesser/flutter/blob/master/packages/flutter/lib/src/widgets/navigation_toolbar.dart?

if isn't different or customized in any way, we should use NavigationToolbar directly instead of copying the source code and maintaining it.
If you need to call it WoltNavigationToolbar and not NavigationToolbar, we can return NavigationToolbar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@TahaTesser As mentioned in here, the NavigationToolBar doesn't center the middle in case the leading or trailing is missing. Here is the change

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense if we've modifications.

Comment on lines 2 to +9
import 'package:wolt_modal_sheet/src/content/components/current/current_main_content_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/current/current_sab_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/current/current_top_bar_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/current/current_top_bar_controls_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/current/current_navigation_toolbar_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/outgoing/outgoing_main_content_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/outgoing/outgoing_sab_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/outgoing/outgoing_top_bar_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/outgoing/outgoing_top_bar_controls_animated_builder.dart';
import 'package:wolt_modal_sheet/src/content/components/outgoing/outgoing_navigation_toolbar_animated_builder.dart';
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the future we should streamline the imports, this is getting very long and wide.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You should have access to create a card in this board. https://github.com/orgs/woltapp/projects/1/views/1 Can you elaborate your suggestion or specs in the board as card or issue so that someone else can contribute?

@ulusoyca ulusoyca merged commit 73266be into main Aug 7, 2023
@alirp88 alirp88 deleted the use-navigation-toolbar-for-navigation-controls branch January 19, 2024 10:40
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.

3 participants