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

Added docs for the new Web Navigation API #822

Merged
merged 1 commit into from
Dec 8, 2024
Merged

Added docs for the new Web Navigation API #822

merged 1 commit into from
Dec 8, 2024

Conversation

arkivanov
Copy link
Owner

@arkivanov arkivanov commented Dec 7, 2024

Summary by CodeRabbit

  • New Features

    • Introduced the Web Navigation API as an advanced replacement for the previous Web Browser History API, enhancing URL and history management.
    • Added support for various navigation models: Child Stack, Child Pages, and Child Panels.
  • Documentation

    • Updated the browser-history.md document for clarity and added warnings about the API's future.
    • Created comprehensive documentation for the Web Navigation API, including code examples and implementation details.
    • Added a new navigation section in the documentation for easier access to Web Navigation content.

Copy link

coderabbitai bot commented Dec 7, 2024

Walkthrough

The changes in this pull request involve updates to the documentation for the Web Browser History API and the introduction of the new Web Navigation API. The browser-history.md file has been revised for clarity, with warnings about the API's future removal. The web-navigation.md file documents the new API, detailing its interfaces and usage. Additionally, the mkdocs.yml file has been modified to include a new navigation entry for the Web Navigation API documentation.

Changes

File Path Change Summary
docs/navigation/stack/browser-history.md Updated title for consistency, added warning about future removal, restructured content for clarity.
docs/navigation/web-navigation.md Introduced new documentation for the Web Navigation API, detailing interfaces and usage examples.
mkdocs.yml Added new navigation entry for the Web Navigation documentation.

Possibly related PRs

  • Added docs for Child Panels #788: The changes in the main PR regarding the Web Navigation API are related to the addition of documentation for the Child Panels, as both involve enhancements to navigation models and their respective documentation clarity.

🐇 In the realm of code, a new path we weave,
With Web Navigation, we dare to believe.
History's old tale, now bright and anew,
With warnings and guidance, we share what is true.
Hop along, dear devs, through the changes we go,
Embracing the future, let our knowledge grow! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (5)
docs/navigation/web-navigation.md (5)

3-5: Consider enhancing the experimental warning.

While the warning is good, consider adding more context about potential API changes and migration recommendations.

 !!!warning
 
-    This API is experimental, available since version `3.3.0-alpha01`.
+    This API is experimental, available since version `3.3.0-alpha01`. As an experimental API,
+    it may undergo breaking changes in future releases. Please provide feedback and report any
+    issues you encounter to help stabilize the API.

96-96: Enhance TODO placeholders with examples.

The TODOs for initialization using deepLinkUrl would be more helpful with basic examples or guidelines.

Consider adding comments like:

-    initialStack = { TODO("Use the deepLinkUrl parameter to initialize the stack") },
+    initialStack = {
+        // Example: Parse the deepLinkUrl to determine initial navigation state
+        // deepLinkUrl?.removePrefix("/") ?: "home" -> Config.Home
+        TODO("Use the deepLinkUrl parameter to initialize the stack")
+    },

Also applies to: 152-152, 206-206


28-28: Add clarification about nested navigation limitations.

The statement about nested navigation could be clearer about the implications and any potential workarounds.

-Only one instance of the `WebNavigation` controller is allowed per component. The new API also supports nested navigation, there can only be up to one (zero or one) child `WebNavigationOwner` at a time.
+Only one instance of the `WebNavigation` controller is allowed per component. While the API supports nested navigation, each component can have at most one child `WebNavigationOwner` at any time. This ensures proper coordination between parent and child navigation states.

34-34: Consider adding error handling section.

The documentation would benefit from a section about handling navigation errors and edge cases.

Consider adding a new section that covers:

  • Handling invalid URLs or deep links
  • Managing navigation state conflicts
  • Error recovery strategies
  • Browser compatibility considerations

233-233: Enhance the example section.

The reference to the main sample could be more helpful with specific file links and key points to look for.

-Please refer to the [main sample](https://github.com/arkivanov/Decompose/tree/master/sample) for a complete example of using the Web Navigation API.
+Please refer to the [main sample](https://github.com/arkivanov/Decompose/tree/master/sample) for a complete example of using the Web Navigation API. Key implementation examples can be found in:
+- `sample/app-js/src/jsMain/kotlin/com/arkivanov/sample/app/root/DefaultRootComponent.kt` - Root component setup
+- `sample/app-js/src/jsMain/kotlin/com/arkivanov/sample/app/App.kt` - Web Navigation initialization
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 828d5b5 and 663f9df.

📒 Files selected for processing (3)
  • docs/navigation/stack/browser-history.md (1 hunks)
  • docs/navigation/web-navigation.md (1 hunks)
  • mkdocs.yml (1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/navigation/web-navigation.md

49-49: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


60-60: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


75-75: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


121-121: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


131-131: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


175-175: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


185-185: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)

🔇 Additional comments (2)
docs/navigation/stack/browser-history.md (1)

3-5: LGTM! Clear deprecation notice.

The warning effectively communicates the API's future removal and properly directs users to the new Web Navigation API documentation.

mkdocs.yml (1)

52-52: LGTM! Well-placed navigation entry.

The new Web Navigation entry is properly positioned within the Navigation section, maintaining the documentation's logical structure.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (5)
docs/navigation/web-navigation.md (5)

3-6: Enhance version information in the warning section

Consider adding more specific information about:

  • The expected stability timeline
  • Compatibility with the old API during transition
  • Minimum version requirements for different features

Would you like me to propose an enhanced warning section?


28-28: Add clarification about WebNavigation constraints

The statement about navigation constraints could be clearer. Consider adding:

  • Examples of incorrect configurations
  • Explanation of why these constraints exist
  • Diagrams showing valid/invalid nested navigation setups

Would you like me to propose expanded documentation for this section?


38-39: Expand deployment configuration section

The catch-all strategy section could benefit from:

  • Examples for common deployment platforms (Nginx, Apache, etc.)
  • Troubleshooting guide for common issues
  • Security considerations for different configurations

Would you like me to provide examples for different deployment scenarios?


96-96: Enhance implementation guidance in examples

The TODOs would be more helpful with:

  • Concrete examples of typical implementations
  • Best practices for each customization point
  • Error handling scenarios

Would you like me to provide example implementations for these TODOs?

Also applies to: 107-109


231-233: Enhance sample reference section

Consider expanding this section with:

  • Overview of what the sample demonstrates
  • Key implementation patterns to observe
  • Common pitfalls the sample helps avoid

Would you like me to propose an expanded sample reference section?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 663f9df and 197f2ff.

📒 Files selected for processing (3)
  • docs/navigation/stack/browser-history.md (1 hunks)
  • docs/navigation/web-navigation.md (1 hunks)
  • mkdocs.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • mkdocs.yml
  • docs/navigation/stack/browser-history.md
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/navigation/web-navigation.md

49-49: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


60-60: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


75-75: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


121-121: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


131-131: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


175-175: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


185-185: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (4)
docs/navigation/web-navigation.md (4)

13-13: Improve sentence clarity

The sentence structure could be clearer. Consider revising to:
"The Child Slot navigation model is currently not supported. It will likely require additional API changes and might be implemented in the future."

🧰 Tools
🪛 LanguageTool

[uncategorized] ~13-~13: A determiner appears to be missing. Consider inserting it.
Context: ... likely require additional API changes, might be implemented in the future. ## WebNa...

(AI_EN_LECTOR_MISSING_DETERMINER)


28-28: Consider adding error handling information

The documentation mentions the constraint of "only one instance per component" but doesn't specify what happens if this constraint is violated. Consider adding information about error handling or validation mechanisms.


38-39: Add production deployment configuration examples

While the development configuration with webpack-dev-server is well explained, consider adding examples or guidance for production deployment configurations (e.g., nginx, Apache, or other common web servers).


231-232: Enhance sample code reference

Consider adding more specific guidance about what aspects of the Web Navigation API implementation to look for in the main sample, such as:

  • Deep linking implementation examples
  • Navigation state management
  • Integration with different navigation models
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 197f2ff and fdd9293.

📒 Files selected for processing (3)
  • docs/navigation/stack/browser-history.md (1 hunks)
  • docs/navigation/web-navigation.md (1 hunks)
  • mkdocs.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • mkdocs.yml
  • docs/navigation/stack/browser-history.md
🧰 Additional context used
🪛 LanguageTool
docs/navigation/web-navigation.md

[uncategorized] ~13-~13: A determiner appears to be missing. Consider inserting it.
Context: ... likely require additional API changes, might be implemented in the future. ## WebNa...

(AI_EN_LECTOR_MISSING_DETERMINER)

🪛 Markdownlint (0.35.0)
docs/navigation/web-navigation.md

49-49: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


60-60: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


75-75: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


121-121: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


131-131: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


175-175: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)


185-185: Expected: indented; Actual: fenced
Code block style

(MD046, code-block-style)

@arkivanov arkivanov merged commit 363192a into master Dec 8, 2024
3 checks passed
@arkivanov arkivanov deleted the update-docs branch December 8, 2024 17:03
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.

1 participant