fix: Added examples of individual coachmarks with UI for readme#7
fix: Added examples of individual coachmarks with UI for readme#7
Conversation
WalkthroughThis update migrates the project to .NET 9, introduces new sample infrastructure with a base page and view model, and refactors coachmark functionality to use CommunityToolkit.Maui popups instead of modal pages. It adds a new focus coachmark sample, centralizes resource constants, revises styles and colors, enhances platform-specific view measurement logic, and simplifies coachmark positioning by removing diagonal options. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainPage
participant MainViewModel
participant App
participant FocusCoachmarkPage
participant CoachmarkManager
participant FreakyPopupPage (Popup)
User->>MainPage: Tap animation type
MainPage->>MainViewModel: ShowNextPageCommand(selection)
MainViewModel->>App: Navigate to FocusCoachmarkPage (if Focus)
App->>FocusCoachmarkPage: Push page onto navigation stack
FocusCoachmarkPage->>CoachmarkManager: Start coachmark tutorial
CoachmarkManager->>FreakyPopupPage (Popup): ShowPopupAsync with options
User->>FreakyPopupPage (Popup): Interact with coachmark overlays
FreakyPopupPage (Popup)->>CoachmarkManager: NextCoachMark or ClosePopupAsync
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (1)
67-74: 🛠️ Refactor suggestionAvoid repeated
IEnumerableenumeration
_views.Count()&ElementAtOrDefaultre-enumerate every call.
Convert once to aList<View>for O(1) access.- _views = coachMarkViews; + _views = coachMarkViews?.ToList() ?? new List<View>();Then update field type to
IReadOnlyList<View>(orList<View>) and replaceCount()with.Count.
🧹 Nitpick comments (13)
Maui.FreakyUXKit/Samples/Resources/Styles/Colors.xaml (2)
8-8: Spelling inconsistency: “Quarternary” → “Quaternary”The term is normally spelled Quaternary. Keeping resource keys typo-free avoids future lookup errors.
-<Color x:Key="Quarternary">#F5F5F5</Color> +<Color x:Key="Quaternary">#F5F5F5</Color>
21-33: Missing matching brush for the new colourAll base colours except
Quaternaryhave an accompanying brush, which breaks the established convention and forces consumers to create their own.+<SolidColorBrush x:Key="QuaternaryBrush" Color="{StaticResource Quaternary}" />Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkViewModel.cs (1)
1-6: Empty ViewModel – opportunity to seal intentRight now the class adds no behaviour beyond
FreakyBaseViewModel.
If it is merely a marker, mark itsealedand add an XML summary so future contributors know it is intentionally blank.-public class FocusCoachmarkViewModel : FreakyBaseViewModel +/// <summary> +/// View-model for <see cref="FocusCoachmarkPage"/>. +/// Currently inherits navigation helpers from <see cref="FreakyBaseViewModel"/>. +/// </summary> +public sealed class FocusCoachmarkViewModel : FreakyBaseViewModelMaui.FreakyUXKit/Samples/MauiProgram.cs (1)
2-3: Using directives can be file-scopedMinor: you can convert the
usingstatements to file-scoped (global using) in a central file to reduce repetition across sample projects.Maui.FreakyUXKit/Samples/AppShell.xaml.cs (1)
1-9: No navigation route registrationIf additional pages will be pushed from code, register routes in the constructor:
Routing.RegisterRoute(nameof(FocusCoachmarkPage), typeof(FocusCoachmarkPage));Helps avoid magic strings in future navigation calls.
Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml.cs (1)
5-9: Inject ViewModel via DI instead of manually new-ing it upHard-wiring the ViewModel (
new FocusCoachmarkViewModel()) couples the page to a concrete implementation and complicates unit‐testing or later replacement. Resolve the VM fromIServiceProvider(MAUI supports constructor injection) or assign it in XAML to keep the code-behind clean.-public FocusCoachmarkPage() -{ - InitializeComponent(); - BindingContext = new FocusCoachmarkViewModel(); -} +public FocusCoachmarkPage(FocusCoachmarkViewModel vm) +{ + InitializeComponent(); + BindingContext = vm; +}Then register the VM in
MauiProgram:builder.Services.AddTransient<FocusCoachmarkViewModel>();Maui.FreakyUXKit/Samples/AppShell.xaml (1)
8-11: Route naming mismatch might confuse deep-linking
Route="MainPage"points toFocusCoachmarkPage. Because a different page called “MainPage” often exists, this can cause ambiguity when using URI-based navigation. Prefer a route that reflects the actual target, e.g.focus-coachmark.-Route="MainPage" +Route="focus-coachmark"Maui.FreakyUXKit/Samples/Samples.csproj (1)
56-60: Embedded SVGs duplicated asMauiImage?The same SVGs appear to live under
Resources/Images. They’re now embedded resources, but if they’re also picked up by the<MauiImage Include="Resources\Images\*"/>glob you’ll ship each file twice, increasing app size. Either:
- Exclude them from the image glob, or
- Reference them only as images and drop the explicit
EmbeddedResource.Verify which mechanism the controls expect.
Maui.FreakyUXKit/Maui.FreakyUXKit/ViewExtensions.cs (1)
35-58: Cachedensity& reduce property-lookups
nativeView.Context.Resources.DisplayMetrics.Densityis queried four times.
Caching it once improves readability and avoids redundant JNI hops.- double width = nativeView.Width / nativeView.Context.Resources.DisplayMetrics.Density; - double height = nativeView.Height / nativeView.Context.Resources.DisplayMetrics.Density; - - x /= nativeView.Context.Resources.DisplayMetrics.Density; - y /= nativeView.Context.Resources.DisplayMetrics.Density; + var density = nativeView.Context.Resources.DisplayMetrics.Density; + + double width = nativeView.Width / density; + double height = nativeView.Height / density; + + x /= density; + y /= density;(While you’re here: the class name
ViewExntensionsis miss-spelt; consider correcting toViewExtensionsin a follow-up PR.)Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (1)
45-51: Double-checkingLoadedsubscription logic
Loadedis unsubscribed inOnDisappearing().
If the same page instance is ever re-used (e.g., with Shell’s caching), the coach-mark will no longer display.
Consider either:
- keep the subscription for the lifetime of the page (most common), or
- resubscribe in
OnAppearing()instead.Optional but worth validating against your navigation flow.
Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml (1)
36-61: Add accessibility labels to interactive header buttonsScreen-reader users currently receive no description for the back/menu buttons.
AttachAutomationProperties.Name(and optionallyHelpText) for better a11y:- <controls:FreakySvgImageView + <controls:FreakySvgImageView + AutomationProperties.Name="Back" ImageColor="White" @@ - <controls:FreakySvgImageView + <controls:FreakySvgImageView + AutomationProperties.Name="Context menu" ImageColor="White"Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml (1)
70-157: Three nearly-identical metric panels ⇒ extract to aDataTemplate
Students,Content,Followersblocks are copy-pasted. This violates DRY and inflates XAML compile time.Refactor to:
- An
ObservableCollection<MetricVm>on the VM.- A
CollectionViewwith a reusableMetricTemplate.This keeps layout consistent and makes future changes (icon size, font) one-liner updates.
Maui.FreakyUXKit/Samples/Resources/Styles/Styles.xaml (1)
404-407: Page Padding set to zero may overlap iOS status bar
Removing padding everywhere looks fine on Android but pushes content under the notch on iOS/macOS Catalyst.Consider:
-<Setter Property="Padding" Value="0" /> +<Setter Property="Padding" Value="{OnPlatform iOS='0,20,0,0', Default='0'}" />Or leverage
SafeAreaInsetsto keep headers readable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
Maui.FreakyUXKit/Samples/Resources/Images/back_button.svgis excluded by!**/*.svgMaui.FreakyUXKit/Samples/Resources/Images/bar_chart.svgis excluded by!**/*.svgMaui.FreakyUXKit/Samples/Resources/Images/briefcase.pngis excluded by!**/*.pngMaui.FreakyUXKit/Samples/Resources/Images/meatball_icon.svgis excluded by!**/*.svgMaui.FreakyUXKit/Samples/Resources/Images/play.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Maui.FreakyUXKit/Maui.FreakyUXKit.csproj(1 hunks)Maui.FreakyUXKit/Maui.FreakyUXKit/ViewExtensions.cs(1 hunks)Maui.FreakyUXKit/Samples/App.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/AppShell.xaml(1 hunks)Maui.FreakyUXKit/Samples/AppShell.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/Constants.cs(1 hunks)Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/MainPage.xaml(0 hunks)Maui.FreakyUXKit/Samples/MainPage.xaml.cs(0 hunks)Maui.FreakyUXKit/Samples/MauiProgram.cs(2 hunks)Maui.FreakyUXKit/Samples/Resources/Styles/Colors.xaml(2 hunks)Maui.FreakyUXKit/Samples/Resources/Styles/Styles.xaml(13 hunks)Maui.FreakyUXKit/Samples/Samples.csproj(1 hunks)
💤 Files with no reviewable changes (2)
- Maui.FreakyUXKit/Samples/MainPage.xaml.cs
- Maui.FreakyUXKit/Samples/MainPage.xaml
🧰 Additional context used
🧬 Code Graph Analysis (3)
Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkViewModel.cs (1)
Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs (2)
FreakyBaseViewModel(6-20)FreakyBaseViewModel(10-13)
Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml.cs (1)
Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkViewModel.cs (1)
FocusCoachmarkViewModel(3-6)
Maui.FreakyUXKit/Samples/App.xaml.cs (1)
Maui.FreakyUXKit/Samples/AppShell.xaml.cs (2)
AppShell(3-9)AppShell(5-8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (11)
Maui.FreakyUXKit/Samples/Resources/Styles/Colors.xaml (1)
5-10: Use 6- or 8-digit ARGB notation for better MAUI compatibilityMAUI reliably recognises
#RRGGBBand#AARRGGBBliterals.
#000/#FFFrely on shorthand parsing that is not guaranteed across all targets and colour pickers.
Consider expanding them to explicit 24-bit (or 32-bit) representations, e.g.:-<Color x:Key="Primary">#000</Color> -<Color x:Key="Secondary">#FFF</Color> +<Color x:Key="Primary">#000000</Color> +<Color x:Key="Secondary">#FFFFFF</Color>Maui.FreakyUXKit/Maui.FreakyUXKit/Maui.FreakyUXKit.csproj (1)
34-44: Confirm removal of<Folder>items doesn’t break linker trimmingThe deleted
<Folder>declarations only serve IntelliSense; however, if embedded resources or platform-specific partial classes were previously kept alive by their presence, the linker might now remove them. Please verify that:
- Any
*.axml,*.storyboard, SVG, etc., that relied on those folders are still marked with the correctBuildAction.- Platform-specific partials continue to compile after the physical folder move.
Maui.FreakyUXKit/Samples/MauiProgram.cs (1)
15-17: Order of builder extensions – verify toolkit initialisationSome MAUI extensions expect
UseMauiCommunityToolkit()to run before custom libraries that register handlers (e.g. media element). IfFreakyUXKitinternally registers handlers that rely on the toolkit, swap the calls:- .UseFreakyUXKit() - .UseMauiCommunityToolkit() + .UseMauiCommunityToolkit() + .UseFreakyUXKit()Please confirm the required order, otherwise runtime handler registration could fail silently.
Maui.FreakyUXKit/Samples/App.xaml.cs (1)
8-8: Confirm Shell meets multi-window requirementsReplacing
CreateWindow()with a directMainPage = new AppShell();is fine for single-window scenarios, but .NET MAUI on macOS/Windows supports multi-window viaCreateWindow. If the sample is ever expected to open additional windows, consider retaining/overridingCreateWindowand returning aWindowthat hostsAppShellto avoid future refactors.Maui.FreakyUXKit/Samples/Samples.csproj (1)
49-55: Align MAUI & toolkit package versions
CommunityToolkit.Maui 9.1.1targets MAUI 8.0.82+, so the versions are compatible, but if you update MAUI in one place (8.0.82) remember to bump theMicrosoft.Maui.Extensions.*meta-packages too (if present) to keep transitive dependencies consistent. A skew can lead to binding/runtime warnings.Maui.FreakyUXKit/Samples/Constants.cs (1)
5-20: Centralised constants look goodThe constant set is clear and self-explanatory. No issues spotted.
Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml.cs (1)
1-96: Bindable-property scaffold looks solidNo functional issues observed; defaults make sense and naming is consistent.
Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml (2)
268-282: Coachmark overlay is missing DisplayOrder / OwningPage – may never show
fc:FreakyCoachmark.OverlayViewusually requiresDisplayOrderand sometimesOwningPageto participate in the coachmark sequence. Only the image (lines 333-336) declares these.If the intention is that this card participates in the same flow, add:
<fc:FreakyCoachmark.OverlayView + fc:FreakyCoachmark.DisplayOrder="2" + fc:FreakyCoachmark.OwningPage="{Reference page}">Otherwise the overlay is ignored by the coachmark engine.
333-336: 👍 Correct usage of animation & ordering attributes
The image correctly setsCoachmarkAnimation,DisplayOrder, andOwningPage; this will compile-time validate thanks tox:DataType.Maui.FreakyUXKit/Samples/Resources/Styles/Styles.xaml (2)
48-62: Verify colour key spelling – “Quarternary” vs “Quaternary”
SecondaryButtonStyle/QuarternaryButtonStylereferenceQuarternary(with an extra ‘r’). If yourColors.xamldefinesQuaternary, this will throw a XAML parse exception at run-time.Please ensure the resource exists or rename consistently.
410-419: Shell foreground/background contrast in Light theme
Shell.BackgroundColorusesPrimary(likely dark) whileShell.ForegroundColorfalls back to white on most platforms, resulting in low contrast ifPrimaryis light. Double-check WCAG contrast ratios or swap colours to maintain 4.5:1.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (1)
57-60: RemoveConfigureAwait(false)– still risks UI-thread violations
This was flagged previously and remains unresolved.NextCoachMark()touches UI elements; continuation must stay on the main thread.- await NextCoachMark().ConfigureAwait(false); + await NextCoachMark();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/CoachmarkManager.cs(1 hunks)Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml(2 hunks)Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs(3 hunks)Maui.FreakyUXKit/Maui.FreakyUXKit/Maui.FreakyUXKit.csproj(2 hunks)Maui.FreakyUXKit/Samples/Samples.csproj(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Maui.FreakyUXKit/Maui.FreakyUXKit/Maui.FreakyUXKit.csproj
🧰 Additional context used
🧬 Code Graph Analysis (1)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/CoachmarkManager.cs (3)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
FreakyPopupPage(9-578)FreakyPopupPage(43-49)Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyCoachmark.cs (2)
FreakyCoachmark(5-268)GetDisplayOrder(208-209)Maui.FreakyUXKit/Maui.FreakyUXKit/Constants.cs (1)
Constants(4-14)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (3)
Maui.FreakyUXKit/Samples/Samples.csproj (2)
4-4: Targeting .NET 9 preview – verify toolchain readiness
net9.0-android;net9.0-iosrequires the .NET 9 preview SDK + matching MAUI workload.
Please confirm CI agents / local dev boxes use the same preview; otherwise builds will fail.
48-52: Confirm upgraded package set & transitive toolkit usageUpgrading to
Microsoft.Maui.Controls 9.0.80and addingFreakyControls/CommunityToolkit.Mvvmis fine, but be sure:
- Any removed
CompatibilityAPI calls were really eliminated.- The
CommunityToolkit.Mauipackage (needed forPopup) is available transitively; if not, add it explicitly to avoid missing-type compile errors.A quick restore/build run will surface any gaps.
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml (1)
2-8: XAML root update looks goodThe conversion to
toolkit:Popupwith proper namespaces is correct; no further action needed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
56-59: RemoveConfigureAwait(false)— causes UI-thread violations
NextCoachMark()manipulatesNavigationand UI elements and must execute on the main thread.- await NextCoachMark().ConfigureAwait(false); + await NextCoachMark();
42-47: Materialise_viewsto avoid repeated enumeration
_views.Count()and multipleElementAtOrDefaultcalls re-enumerate the source each time.- _views = coachMarkViews; + _views = coachMarkViews.ToList();
🧹 Nitpick comments (1)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (1)
1-10: Fix PR title typoThe PR title contains a typo: "shell exampels" should be "shell examples". Additionally, no Shell-related changes are visible in the reviewed files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs(13 hunks)Maui.FreakyUXKit/Samples/App.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/MainPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/MainPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/MainViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/Samples.csproj(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs
- Maui.FreakyUXKit/Samples/Samples.csproj
- Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (2)
Maui.FreakyUXKit/Samples/MainPage.xaml.cs (1)
1-10: LGTM!The simplified implementation with direct BindingContext assignment is clean and appropriate.
Maui.FreakyUXKit/Samples/MainPage.xaml (1)
1-39: LGTM!The CollectionView implementation with proper data binding and command handling is well-structured.
FreakyAli
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
20-23: Remove trailing punctuation and reconsider heading usage
The## Previews:heading has a trailing colon (MD026) and the### Note: ...line is formatted as a heading ending with a period (also flagged by MD026).
- Change
## Previews:to## Previews- Convert the note into a plain paragraph or italic block without trailing punctuation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~22-~22: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...don't represent the actual performance. Please feel free to clone the repository and check the perf...
(FEEL_FREE_TO_STYLE_ME)
🪛 markdownlint-cli2 (0.17.2)
README.md
20-20: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
22-22: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
26-26: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
26-26: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (2)
README.md (2)
46-47: Installation command wording is clear
The instruction to "Run the following command to add our NuGet to your .NET MAUI app:" uses correct casing forNuGetand.NET MAUIand reads clearly.
50-51: Using statement initialization is accurate
The prose “Add the following using statement and initialization in your MauiProgram:” correctly directs users and matches the code snippet below.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
56-59: RemoveConfigureAwait(false)— causes UI-thread violationsThis is the same issue identified in previous reviews.
NextCoachMark()manipulatesNavigationand UI elements and must execute on the main thread.ConfigureAwait(false)detaches the continuation from the UI context, riskingInvalidOperationExceptionon some platforms.- await NextCoachMark().ConfigureAwait(false); + await NextCoachMark();
42-47: Materialise_viewsto avoid repeated enumerationThe same issue from previous reviews persists.
_views.Count()and multipleElementAtOrDefaultcalls re-enumerate the source each time. Convert to a list once in the constructor:- _views = coachMarkViews; + _views = coachMarkViews.ToList();This guarantees stable ordering and O(1) count/look-ups.
🧹 Nitpick comments (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
493-563: Optimize overlay measurement with cachingThe
MeasureOverlayView()method performs complex calculations every time it's called. Consider caching the measurement result since the overlay view and container dimensions typically don't change between calls during the same coachmark display.+ private Size? _cachedOverlaySize; + private View _lastMeasuredOverlay; + private Size MeasureOverlayView() { if (OverlayView == null) return new Size(200, 100); + + // Return cached size if overlay hasn't changed + if (_cachedOverlaySize.HasValue && _lastMeasuredOverlay == OverlayView) + return _cachedOverlaySize.Value; // ... existing measurement logic ... + // Cache the result + _cachedOverlaySize = finalSize; + _lastMeasuredOverlay = OverlayView; + return finalSize; }
739-806: Consider extracting complex measurement logicThe
MeasureOverlayViewWithScreenConstraintsmethod is quite long and handles multiple responsibilities. Consider extracting the constraint calculation logic into separate methods for better maintainability.private Size MeasureOverlayViewWithScreenConstraints() { if (OverlayView == null) return new Size(200, 100); - // Get actual container/screen dimensions - var containerWidth = container.Width > 0 ? container.Width : Constants.Width; - var containerHeight = container.Height > 0 ? container.Height : Constants.Height; - - // Add some padding to prevent overlay from touching screen edges - var screenPadding = 10; - var maxScreenWidth = containerWidth - (screenPadding * 2); - var maxScreenHeight = containerHeight - (screenPadding * 2); + var screenConstraints = CalculateScreenConstraints(); // ... rest of method using screenConstraints ... } +private (double maxWidth, double maxHeight, double containerWidth, double containerHeight) CalculateScreenConstraints() +{ + var containerWidth = container.Width > 0 ? container.Width : Constants.Width; + var containerHeight = container.Height > 0 ? container.Height : Constants.Height; + var screenPadding = 10; + + return ( + containerWidth - (screenPadding * 2), + containerHeight - (screenPadding * 2), + containerWidth, + containerHeight + ); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs(10 hunks)Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
208-236: Enhanced animation rendering structureThe refactored
RenderFocusAnimationmethod shows improved organization with clearer phase separation and better drawing order. The logic ensures base elements are always drawn before adding effects, which prevents rendering artifacts.
483-491: Verify animation timer disposal in all scenariosThe
StopFocusAnimationmethod properly disposes the timer, but ensure this is called in all cleanup scenarios including unexpected popup closure.#!/bin/bash # Description: Verify that StopFocusAnimation is called in all cleanup paths # Search for all methods that might need to call StopFocusAnimation ast-grep --pattern $'class $_ { $$$ StopFocusAnimation() { $$$ } $$$ }' # Look for potential cleanup methods that should call StopFocusAnimation rg -A 5 -B 2 "StopFocusAnimation|StopAnimation|Cleanup|Dispose|Close"
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
26-27: Add descriptive alt text to preview images.
The<img>tags in the previews table lackaltattributes, which is required for accessibility (MD045).Apply this diff to include meaningful alt text:
-| <img src="https://github.com/user-attachments/assets/c060fc82-ecef-48af-84b4-e07a963e65d5" width="250" height="550"/> +| <img src="https://github.com/user-attachments/assets/c060fc82-ecef-48af-84b4-e07a963e65d5" alt="iOS coachmark preview" width="250" height="550"/> -| <img src="https://github.com/user-attachments/assets/6eabce77-dcf3-46fc-b404-391a4778a6b5" width="250" height="550"/> +| <img src="https://github.com/user-attachments/assets/6eabce77-dcf3-46fc-b404-391a4778a6b5" alt="Android coachmark preview" width="250" height="550"/> -| <img src="https://github.com/user-attachments/assets/402b57d1-a264-4aff-a91b-d0b9e21e81e6" width="250" height="550"/> +| <img src="https://github.com/user-attachments/assets/402b57d1-a264-4aff-a91b-d0b9e21e81e6" alt="iOS coachmark sample 2" width="250" height="550"/> -| <img src="https://github.com/user-attachments/assets/e076fca2-2b31-4e18-a568-ec62ea2d42ee" width="250" height="550"/> +| <img src="https://github.com/user-attachments/assets/e076fca2-2b31-4e18-a568-ec62ea2d42ee" alt="Android coachmark sample 2" width="250" height="550"/>
🧹 Nitpick comments (3)
README.md (3)
20-20: Remove trailing punctuation in heading.
The colon at the end of the## Previews:heading should be dropped to comply with Markdown style (MD026).
22-22: Remove trailing punctuation in subheading.
The period at the end of the### Note: …heading should be removed to comply with Markdown style (MD026).
37-37: Remove duplicate empty "Previews" section.
There’s a second## Previews:heading with no content below it—please remove this redundant section.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~22-~22: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...don't represent the actual performance. Please feel free to clone the repository and check the perf...
(FEEL_FREE_TO_STYLE_ME)
🪛 markdownlint-cli2 (0.17.2)
README.md
20-20: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
22-22: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
26-26: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
26-26: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
27-27: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
27-27: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
Maui.FreakyUXKit/Samples/Spotlight/SpotlightCoachmarkPage.xaml (1)
1-395: Consider refactoring to reduce code duplication.This page is nearly identical to
ArrowCoachmarkPage.xamlwith only the animation type changed. Consider extracting the common layout structure into a shared template or base class to improve maintainability and reduce duplication.Potential approaches:
- Create a shared ControlTemplate for the common layout
- Use a base page class with configurable coachmark properties
- Implement a single page that switches animation types based on a parameter
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
Maui.FreakyUXKit/Samples/App.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/AppShell.xaml(1 hunks)Maui.FreakyUXKit/Samples/AppShell.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/MainViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/Spotlight/SpotlightCoachmarkPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/Spotlight/SpotlightCoachmarkPage.xaml.cs(1 hunks)Maui.FreakyUXKit/Samples/Spotlight/SpotlightViewModel.cs(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- Maui.FreakyUXKit/Samples/Spotlight/SpotlightViewModel.cs
- Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkViewModel.cs
- Maui.FreakyUXKit/Samples/AppShell.xaml
- Maui.FreakyUXKit/Samples/Spotlight/SpotlightCoachmarkPage.xaml.cs
- Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkPage.xaml.cs
🚧 Files skipped from review as they are similar to previous changes (6)
- Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs
- Maui.FreakyUXKit/Samples/App.xaml.cs
- Maui.FreakyUXKit/Samples/MainViewModel.cs
- Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml.cs
- Maui.FreakyUXKit/Samples/FreakyBaseContentPage.xaml
- Maui.FreakyUXKit/Samples/Focus/FocusCoachmarkPage.xaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (2)
Maui.FreakyUXKit/Samples/AppShell.xaml.cs (1)
1-15: Clean routing implementation following MAUI best practices.The AppShell implementation is well-structured with const route names preventing magic strings and proper route registration. The code follows standard MAUI Shell patterns correctly.
Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkPage.xaml (1)
1-400: Comprehensive coachmark demonstration with good XAML structure.The XAML implementation effectively demonstrates various arrow coachmark configurations with proper:
- Data binding and view model integration
- Resource usage and styling consistency
- Coachmark property configuration with different arrow styles
- Complex but well-organized layout hierarchy
The page serves as an excellent example for arrow-style coachmarks.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs (1)
27-32: Add error handling for browser operations.Same as the previous method, this also needs error handling for the
Browser.Default.OpenAsynccall.
🧹 Nitpick comments (2)
Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs (2)
9-11: Consider using more specific command types.While the current implementation works correctly, consider using
IAsyncRelayCommandinstead ofICommandfor better type safety and IntelliSense support.- public ICommand BackButtonCommand { get; } - public ICommand HireCommand { get; } - public ICommand FollowCommand { get; } + public IAsyncRelayCommand BackButtonCommand { get; } + public IAsyncRelayCommand HireCommand { get; } + public IAsyncRelayCommand FollowCommand { get; }
22-22: Consider making URLs configurable.The hardcoded URLs could be made configurable through constants or configuration to improve maintainability and testability.
Consider creating a constants class:
public static class ProfileUrls { public const string LinkedIn = "https://www.linkedin.com/in/freakyali/"; public const string GitHub = "https://github.com/FreakyAli"; }Then reference these constants in the methods.
Also applies to: 29-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/Platforms/Android/AndroidManifest.xml(1 hunks)Maui.FreakyUXKit/Samples/Resources/Styles/Styles.xaml(13 hunks)
✅ Files skipped from review due to trivial changes (1)
- Maui.FreakyUXKit/Samples/Platforms/Android/AndroidManifest.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- Maui.FreakyUXKit/Samples/Resources/Styles/Styles.xaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (1)
Maui.FreakyUXKit/Samples/FreakyBaseViewModel.cs (1)
15-17: Good implementation of previous feedback.The implementation correctly uses
AsyncRelayCommandwith async Task methods, addressing the previous review comment about avoiding async void and using proper MVVM patterns.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (1)
850-904: Add error handling for complex coachmark operations.This method performs extensive UI calculations and container manipulations but lacks error handling. Based on past review feedback, edge cases like invalid bounds, measurement failures, or container issues should be handled gracefully.
The previous review comment about adding error handling to
ShowCurrentCoachMarkis still valid and should be addressed to prevent crashes during complex overlay positioning operations.
🧹 Nitpick comments (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
10-10: Update field type for consistency with constructor parameter.The constructor now accepts
List<View>but the field_viewsis still typed asIEnumerable<View>. This creates an unnecessary type mismatch.- private readonly IEnumerable<View> _views; + private readonly List<View> _views;This change would also address the past review comment about repeated enumeration since
List<T>.Countand indexing are O(1) operations.Also applies to: 42-46
493-563: Consider breaking down this complex measurement method.The
MeasureOverlayViewmethod is quite long and handles multiple responsibilities: temporary container management, constraint calculations, position determination, and re-measurement logic.Consider extracting helper methods:
EnsureOverlayInContainer()for temporary container managementCalculateInitialConstraints()for container dimension logicGetConstrainedSize()for the re-measurement logicThis would improve readability and testability of the positioning logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/CoachmarkManager.cs(1 hunks)Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs(10 hunks)Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkViewModel.cs(1 hunks)Maui.FreakyUXKit/Samples/Spotlight/SpotlightCoachmarkPage.xaml(1 hunks)Maui.FreakyUXKit/Samples/Spotlight/SpotlightViewModel.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Maui.FreakyUXKit/Samples/Arrow/ArrowCoachmarkViewModel.cs
- Maui.FreakyUXKit/Samples/Spotlight/SpotlightViewModel.cs
- Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/CoachmarkManager.cs
- Maui.FreakyUXKit/Samples/Spotlight/SpotlightCoachmarkPage.xaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (2)
Maui.FreakyUXKit/Maui.FreakyUXKit/Coachmark/FreakyPopupPage.xaml.cs (2)
56-59: LGTM - ConfigureAwait issue resolved.Good fix! The previous
ConfigureAwait(false)call has been removed, ensuring UI operations continue on the main thread.
208-237: Approve animation rendering improvements.The enhanced focus animation rendering with proper phase management and pulse scaling looks well-implemented. The logic clearly separates the different animation phases and provides smooth transitions.
3 participants
Summary by CodeRabbit
New Features
Improvements
Style
Chores
Documentation