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

Add Custom Settings Bottom Sheet without needing existing code changes #268

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

wolfe719
Copy link

Add method for interested individuals to create custom settings bottom sheets.

This method does not require any code changes to existing projects.

An optional argument is added to the TalkerScreen() function, a settingsBottomSheetCreator function.

If set, the settingsBottomSheetCreator function is called to create a custom Bottom Sheet for settings.

An example project using this method is created - the sample custom settings BottomSheet creator has the three existing basic settings (Enabled, Use console logs, Use history), and adds two more:

  • Custom Bool that is enabled/disabled based upon the "Enabled" setting
  • Global Custom Bool that can be enabled/disabled without regard to the "Enabled" setting

@Frezyx Frezyx added enhancement New feature or request awaiting On the list for consideration or merge labels Nov 12, 2024
@Frezyx
Copy link
Owner

Frezyx commented Nov 12, 2024

Hello @wolfe719 !
Thank you for your contribution! ❤️
I have noticed a lot of changes. I will review it and provide feedback soon.

Sync with Talker Master branch
Changed filenames:

base_card.dart -> talker_base_card.dart
data_card.dart -> talker_data_card.dart
snackbar.dart -> talker_snackbar_content.dart

Moved talker_flutter/lib/src/ui/talker_settings/widgets/talker_setting_card.dart to talker_flutter/lib/src/ui/talker_settings

Renamed class:
BaseBottomSheet -> TalkerBaseBottomSheet

New Exports:
talker_settings.dart
talker_base_card.dart
talker_base_bottom_sheet.dart
talker_data_card.dart
talker_snackbar_content.dart
… creation

If no creator passed into TalkerView - standard settings bottom sheet will be used

Need to pass the settings bottom sheet creator in through TalkerScreen class
This allows easy access to HTTP result’s status codes when logs are collapsed
Example of adding a Custom Settings Bottom Sheet, with custom booleans
Sample uses gradle-8.0-all.zip in gradle-wrapper.properties

Different cached modules get pulled into each project

Need to start back with original sample app source code from upstream

All these to rename sample app with custom settings

Replace shop_app code to match Frezyx/talker shop_app_example

Updated pubspec.lock
@wolfe719
Copy link
Author

wolfe719 commented Dec 9, 2024

Updated to remove conflicts with base branch. Just want to make your review that much easier.

Note that I moved your example app down one directory level, and added a second sample app, this one with a custom settings view. So now you have two example apps:

examples/shop_app - used to be shop_app_example
examples/shop_app_with_custom_settings - new example app demonstrating use of custom settings view

Copy link
Owner

@Frezyx Frezyx left a comment

Choose a reason for hiding this comment

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

@wolfe719 Hello! Thank you so much for this contribution and for your donation! ❤️
But you have changed a lot of things in the core code of the library in order to make it possible to add custom settings.

Also calling talker.notifyListeners() outside of the package leads to this waring:

The member 'notifyListeners' can only be used within instance members of subclasses of 'package:flutter/src/foundation/change_notifier.dart'.

Ability to implement your custom settings in TalkerScreen is really needed.
But it needs to be made much easier.

I suggest opening a new pull request or refining this one to ensure that the changes impact the minimum amount of code possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting On the list for consideration or merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants