-
Notifications
You must be signed in to change notification settings - Fork 15
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
Documentation fixes #129
Documentation fixes #129
Conversation
📝 WalkthroughWalkthroughThis pull request encompasses documentation updates across multiple files in the FlowMVI project. The changes primarily focus on improving documentation clarity, updating configuration files, and enhancing the visual presentation of documentation. Key modifications include updating URLs to HTTPS, adding new metadata, restructuring documentation sections, and refining CSS styles. The updates aim to improve the overall user experience and documentation quality without altering the core functionality of the framework. Changes
Possibly related PRs
Suggested labels
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (11)
docs/docusaurus.config.ts (5)
1-2
: Use consistent import ordering and grouping .
While the changes look fine, consider grouping similar imports (e.g., all from Docusaurus) together for clarity, if not discouraged by house style guidelines.
24-25
: Locale configuration clarity .
Listing the default locale explicitly is good practice. If you ever plan on adding more locales, confirm that the fallback mechanics are documented for future maintainers.
30-56
: Preconnecting and preloading external resources .
These lines significantly improve page load performance, especially for font loading. As a best practice, consider measuring real-world performance (e.g., using Lighthouse) to quantify improvements or detect potential overhead.
Line range hint
81-94
: Color mode defaulting to dark .
You’ve set"dark"
as the default theme. Verify that this aligns with your user base preferences. Some might find a sudden dark UI unexpected.
166-193
: PWA plugin usage .
Your configuration for PWA/manifest is well laid out. If offline usage is essential, consider verifying that all critical assets are referenced underpwaHead
. Do you need a fallback page listed for offline usage?docs/src/css/custom.css (1)
9-10
: Font-face fallback advice .
You have removed WOFF2. If backward compatibility matters, consider providing multiple font formats.docs/docs/state/statemanagement.md (4)
9-9
: Rewording for clarity .
“Fast and durable manner” is fine, but consider removing “extensible” references if they were previously removed to maintain overall documentation consistency.
47-53
: Unordered list indentation .
Static analysis hints note indentation issues for bullet points. Consider aligning them properly to remove lint warnings.- - ... + - ...🧰 Tools
🪛 Markdownlint (0.37.0)
51-51: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
53-53: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
161-166
: “Make a decision” style improvement .
Per static analysis, consider a more concise phrase, e.g., “to decide.”- ...need to read the current state to make a decision on what to do next... + ...need to read the current state to decide what to do next...🧰 Tools
🪛 LanguageTool
[style] ~164-~164: ‘make a decision’ might be wordy. Consider a shorter alternative.
Context: ...e need to read the current state to make a decision on what to do next. In the example abov...(EN_WORDINESS_PREMIUM_MAKE_A_DECISION)
252-253
: Strengthen wording .
Per static analysis, consider replacing “the last drop” with “the last bit” or “maximum performance.”- ... squeeze the last drop of performance ... + ... squeeze the last bit of performance ...🧰 Tools
🪛 LanguageTool
[style] ~252-~252: Consider a different noun to strengthen your wording.
Context: ...ou absolutely must squeeze the last drop of performance from the Store, and you ...(DROP_DECLINE)
README.md (1)
431-431
: Consider cleaning up unused badge referencesThe following badge references are defined but not used in the document:
badge-android-native
badge-js-ir
badge-apple-silicon
Consider either using these badges in the documentation or removing their definitions to maintain clean documentation.
-[badge-android-native]: https://img.shields.io/badge/support-[AndroidNative]-6EDB8D.svg?style=flat -[badge-js-ir]: https://img.shields.io/badge/support-[IR]-AAC4E0.svg?style=flat -[badge-apple-silicon]: https://img.shields.io/badge/support-[AppleSilicon]-43BBFF.svg?style=flatAlso applies to: 437-437, 447-447
🧰 Tools
🪛 Markdownlint (0.37.0)
431-431: Unused link or image reference definition: "badge-android-native"
Link and image reference definitions should be needed(MD053, link-image-reference-definitions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/static/chart.png
is excluded by!**/*.png
📒 Files selected for processing (7)
README.md
(2 hunks)docs/docs/integrations/essenty.md
(1 hunks)docs/docs/plugins/prebuilt.md
(0 hunks)docs/docs/quickstart.md
(2 hunks)docs/docs/state/statemanagement.md
(13 hunks)docs/docusaurus.config.ts
(5 hunks)docs/src/css/custom.css
(5 hunks)
💤 Files with no reviewable changes (1)
- docs/docs/plugins/prebuilt.md
✅ Files skipped from review due to trivial changes (1)
- docs/docs/integrations/essenty.md
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/docs/state/statemanagement.md
51-51: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
53-53: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
README.md
431-431: Unused link or image reference definition: "badge-android-native"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
437-437: Unused link or image reference definition: "badge-js-ir"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
447-447: Unused link or image reference definition: "badge-apple-silicon"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
🪛 LanguageTool
docs/docs/state/statemanagement.md
[style] ~164-~164: ‘make a decision’ might be wordy. Consider a shorter alternative.
Context: ...e need to read the current state to make a decision on what to do next. In the example abov...
(EN_WORDINESS_PREMIUM_MAKE_A_DECISION)
[style] ~252-~252: Consider a different noun to strengthen your wording.
Context: ...ou absolutely must squeeze the last drop of performance from the Store, and you ...
(DROP_DECLINE)
🔇 Additional comments (32)
docs/docusaurus.config.ts (7)
9-18
: Ensure SEO metadata fields are up to date .
Your title
, tagline
, and favicon
references look good. However, verify that these updates align with your broader SEO strategy. Consider adding localized descriptors if you have multiple locales.
27-29
: External fonts: potential offline usage issues .
Relying on external fonts means the documentation might not render correctly offline. Consider bundling your fonts or providing a fallback to ensure consistent user experience in limited connectivity scenarios.
59-66
: Docs configuration consistency .
The doc settings, such as sidebarPath
and routeBasePath
, appear consistent and maintain clarity. Good job!
71-71
: Custom CSS reference .
Linking a single custom CSS appears straightforward and is in line with typical Docusaurus patterns.
140-148
: Additional language syntax highlighting .
Supporting more languages in code blocks is an excellent improvement for developer documentation.
153-159
: Magic comment usage .
Highlighting or error-lining code for docs is a neat feature. Implementation looks correct.
109-131
: Navbar updates and item labeling .
The updated label for your GitHub link is undefined
. Ensure you intend to hide the label, or consider adding a visually hidden text for accessibility.
- label: undefined,
+ label: 'GitHub',
docs/src/css/custom.css (4)
23-24
: Base font family clarity .
Your updated font stack is consistent. This helps unify the look with the newly added Google Fonts.
43-43
: Dark mode highlight color standardization .
Using lowercase hex (#1e2227
) is consistent with typical CSS conventions.
111-111
: Heading font consistency .
Assigning base fonts to certain headings is fine. This can keep styling more consistent across different heading levels.
138-163
: New details and summary styling .
This block meaningfully improves interactive elements and matches the new expansions in docs/quickstart. Great addition!
docs/docs/state/statemanagement.md (17)
Line range hint 13-29
: Encouraging strict immutability .
Your explanation about immutability and the marker interface MVIState
is clear. It reinforces proper state usage.
🧰 Tools
🪛 Markdownlint (0.37.0)
51-51: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
53-53: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
33-33
: Emphasis on avoiding mutable properties .
Reiterating to avoid var
helps ensure the one-way data flow pattern.
39-43
: Empty state availability .
Promoting EmptyState
is helpful for minimal setups. Good documentation clarity.
76-78
: Cast and usage clarity .
Showing the cast to Content
clarifies usage patterns, especially for typed updates.
81-89
: DSL usage for typed updates .
The DSL approach significantly reduces boilerplate. The explanation is well done.
108-109
: Nested states approach .
Clear doc on combining multiple states. It promotes maintainability.
132-132
: Encouragement for null-free states .
Suggesting no “null-ridden code” fosters correctness.
168-171
: Thread and parallel usage highlight .
Reinforcing the library’s concurrency model is beneficial for advanced use cases.
Line range hint 172-190
: StateFlow.update { }
concurrency caution .
This is an important callout. Clarifying potential repeated evaluations helps prevent user-side mistakes.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~202-~202: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...g and long-running code inside the block and it will only be executed once and o...
(COMMA_COMPOUND_SENTENCE)
191-201
: SST explanation .
The concept of serialized transactions is well explained, aligning with best concurrency practices.
Line range hint 211-224
: Nested updates and reentrancy .
Your mention of potential deadlocks is a good warning to developers.
226-227
: Performance note on reentrancy .
1600% slower still negligible for most UIs, but it’s good you mention it explicitly.
229-241
: Non-reentrant strategy clarity .
Providing sample usage with Atomic(reentrant = false)
is valuable.
Line range hint 264-295
: Bypassing locks .
Bypassing locks is advanced usage, as you mention. Validate that each user of updateStateImmediate
is aware of the concurrency pitfalls.
299-300
: Leaks reminder .
Warning about state leakage is a good note for advanced usage.
308-309
: Contrasting MVI with MVVM .
Clarifies the difference between stored hot state versus upstream reactive flows.
333-333
: Persisting data example .
Your snippet preserving search query is helpful.
docs/docs/quickstart.md (2)
8-9
: Imports for ThemedImage and useBaseUrl .
These additions support the new diagram feature. Nicely integrated.
25-37
:
The new diagram section with
<ThemedImage>
is a neat improvement in visual clarity. The alt text is descriptive.
README.md (2)
420-420
: LGTM! Security improvement for license URL
Good update to use HTTPS instead of HTTP for the Apache License URL, following secure web practices.
429-455
: LGTM! Security improvement for badge URLs
Good update to use HTTPS instead of HTTP for all badge image URLs. The changes are consistent across all badge definitions.
🧰 Tools
🪛 Markdownlint (0.37.0)
431-431: Unused link or image reference definition: "badge-android-native"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
437-437: Unused link or image reference definition: "badge-js-ir"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
447-447: Unused link or image reference definition: "badge-apple-silicon"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
Summary by CodeRabbit
Documentation
sidebar_label: Essenty
to Essenty integration documentationStyle