-
Notifications
You must be signed in to change notification settings - Fork 519
feat: ✨ Multiple showcase at the same time and improvement #514
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
Conversation
dce2a62 to
ce0b324
Compare
93f75e4 to
407aa26
Compare
407aa26 to
c55862f
Compare
lib/src/get_position.dart
Outdated
| var renderBox = key.currentContext?.findRenderObject() as RenderBox?; | ||
|
|
||
| void _getRenderBox() { | ||
| final renderBox = context.findRenderObject() as RenderBox?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take the box directly.
lib/src/constants.dart
Outdated
| borderRadius: BorderRadius.all(Radius.circular(8)), | ||
| ); | ||
|
|
||
| static const Widget defaultProgressIndicator = CircularProgressIndicator( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use adaptive version.
lib/src/layout_overlays.dart
Outdated
| void didUpdateWidget(OverlayBuilder oldWidget) { | ||
| super.didUpdateWidget(oldWidget); | ||
| WidgetsBinding.instance.addPostFrameCallback((_) => syncWidgetAndOverlay()); | ||
| if (oldWidget.showOverlay != widget.showOverlay && widget.showOverlay) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please simplify this.
lib/src/layout_overlays.dart
Outdated
| final bool showOverlay; | ||
| final WidgetBuilder? overlayBuilder; | ||
| final Widget child; | ||
| final ValueSetter<VoidCallback> updateOverlay; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please convert this to static method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we convert this to a static method, it is causing an error in the showcase view widget, so we are keeping it the same.
lib/src/showcase_widget.dart
Outdated
| return Container( | ||
| alignment: Alignment.center, | ||
| decoration: BoxDecoration( | ||
| color: overlayColor | ||
|
|
||
| //TODO: Update when we remove support for older version | ||
| //ignore: deprecated_member_use | ||
| .withOpacity(overlayOpacity), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please switch to Align and ColoredBox.
lib/src/showcase_widget.dart
Outdated
| (_) { | ||
| updateOverlay?.call(); | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use expression body. And ensure that this call is needed twice.
lib/src/showcase_widget.dart
Outdated
| } | ||
| } | ||
|
|
||
| void _stopTimer() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be renamed I believe.
ef77c3c to
f5ea659
Compare
f5ea659 to
228af1d
Compare
lib/src/showcase/showcase.dart
Outdated
| showcaseController = ShowcaseController( | ||
| showcaseId: widget.hashCode, | ||
| showcaseKey: widget.showcaseKey, | ||
| showcaseConfig: widget, | ||
| scrollIntoView: _scrollIntoView, | ||
| )..startShowcase = startShowcase; | ||
|
|
||
| showCaseWidgetState.registerShowcaseController( | ||
| controller: showcaseController, | ||
| key: widget.showcaseKey, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep single source of truth.
lib/src/showcase/showcase.dart
Outdated
| ); | ||
| showOverlay(); | ||
| } | ||
| showcaseController.showcaseConfig = widget; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see if this is absolutely required.
lib/src/showcase/showcase.dart
Outdated
|
|
||
| recalculateRootWidgetSize(); | ||
|
|
||
| if (enableShowcase) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please invert this condition and return early.
lib/src/showcase/showcase.dart
Outdated
| const TooltipActionConfig(); | ||
| } | ||
|
|
||
| void _updateControllerData(BuildContext context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep functions testable.
lib/src/showcase_widget.dart
Outdated
| if (!disableBarrierInteraction && | ||
| !firstShowcaseConfig.disableBarrierInteraction) { | ||
| next(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please invert this condition and return early.
lib/src/showcase_widget.dart
Outdated
| if (_showcaseControllers.containsKey(key)) { | ||
| _showcaseControllers[key]!.add(controller); | ||
| } else { | ||
| _showcaseControllers[key] = [controller]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please utilise putIfAbsent method.
lib/src/showcase_widget.dart
Outdated
| final currentControllers = | ||
| (_showcaseControllers[getCurrentActiveShowcaseKey] ?? | ||
| <ShowcaseController>[]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please create a private getter for this.
14bdf3b to
d50a933
Compare
d50a933 to
b476dc8
Compare
lib/src/get_position.dart
Outdated
| var renderBox = key.currentContext?.findRenderObject() as RenderBox?; | ||
| Offset? _boxOffset; | ||
|
|
||
| void _getRenderBox() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this to match the work its doing.
lib/src/layout_overlays.dart
Outdated
| Widget build(BuildContext context) { | ||
| buildOverlay(); | ||
|
|
||
| return widget.child!; | ||
| return widget.child; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this expression body.
lib/src/showcase_widget.dart
Outdated
| //ignore: deprecated_member_use | ||
| .withOpacity(firstShowcaseConfig.overlayOpacity), | ||
| child: const Align( | ||
| alignment: Alignment.center, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be the default parameter please check and remove if so.
lib/src/showcase_widget.dart
Outdated
| if (controllers.length == 1 && | ||
| (controllers.first.showcaseConfig.enableAutoScroll ?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use firstOrNull here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This API has been available since SDK 3.0.0, but our constraints is '>=2.18.0 <4.0.0' so can not use that.
For now added TODO for the same
lib/src/showcase_widget.dart
Outdated
| int showcaseId, | ||
| ) { | ||
| assert( | ||
| _showcaseControllers.containsKey(key) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be avoided.
lib/src/showcase/showcase.dart
Outdated
| showCaseWidgetState | ||
| .getControllerForShowcase( | ||
| widget.showcaseKey, | ||
| _uniqueId, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the created getter for this.
lib/src/showcase/showcase.dart
Outdated
| _globalFloatingActionWidget = showCaseWidgetState | ||
| .globalFloatingActionWidget(widget.showcaseKey) | ||
| ?.call(context); | ||
| final size = _controller.rootWidgetSize ?? MediaQuery.sizeOf(context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ensure compatibility to the lowest version supported by this package.
lib/src/showcase/showcase.dart
Outdated
| Future<void> _nextIfAny() async { | ||
| if (showCaseWidgetState.isShowCaseCompleted) return; | ||
|
|
||
| if (timer != null && timer!.isActive) { | ||
| if (showCaseWidgetState.enableAutoPlayLock) { | ||
| return; | ||
| } | ||
| timer!.cancel(); | ||
| } else if (timer != null && !timer!.isActive) { | ||
| timer = null; | ||
| } | ||
| await _reverseAnimateTooltip(); | ||
| if (showCaseWidgetState.isShowCaseCompleted) return; | ||
| showCaseWidgetState.completed(widget.key); | ||
| showCaseWidgetState.completed(widget.showcaseKey); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can easily be moved to controller. Please check for other such methods and make the move.
3c7679d to
e2721d8
Compare
a70b882 to
01473f3
Compare
e99bb01 to
0a6652b
Compare
0a6652b to
9a94177
Compare
5723af9 to
5405eee
Compare
3a8f599 to
703bfd8
Compare
703bfd8 to
ec0d57c
Compare
related: feat: ✨ Multiple showcase at the same time and improvement SimformSolutionsPvtLtd#514 SimformSolutionsPvtLtd#514
Description
Checklist
fix:,feat:,docs:etc).docsand added dartdoc comments with///.examplesordocs.Breaking Change?
Related Issues
Closes #113