Skip to content

Conversation

@dkonieczek
Copy link
Contributor

@dkonieczek dkonieczek commented Aug 22, 2025

Copilot AI review requested due to automatic review settings August 22, 2025 19:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

The PR performs an initial documentation refactor to restructure and modernize the Snap framework documentation. It consolidates scattered documentation into a more organized hierarchy and removes outdated sections while adding new comprehensive guides.

  • Removes legacy documentation sections including setup, preact-specific guides, and advanced sections
  • Introduces a new structured documentation hierarchy organized by integration type (Snap Integration, API Integration)
  • Consolidates controller documentation by combining README files with additional configuration guides

Reviewed Changes

Copilot reviewed 36 out of 38 changed files in this pull request and generated 5 comments.

File Description
packages/snap-controller/src/*/README.md Removes instantiation examples and code samples from controller documentation
index.html Updates link transformation logic to dynamically generate from documents structure
docs/documents.js Major restructuring of documentation hierarchy, removing old sections and adding new organized categories
docs/*.md Removes outdated files and adds new comprehensive guides for Snap overview, configuration, and usage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dkonieczek dkonieczek requested a review from Copilot October 1, 2025 21:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 76 out of 78 changed files in this pull request and generated 17 comments.

Comments suppressed due to low confidence (3)

doc-app.js:1

  • When a heading lacks an id, createHeadingId returns a synthetic id but it is never assigned to the element. Set heading.id = id when heading.id is falsy so hash navigation and scroll behavior works reliably.
function flattenDocumentLinks(docs) {

docs/SETUP.md:1

  • [nitpick] Typo: 'propmpted' should be 'prompted'. Also consider 'which you can find in the Searchspring Management Console' for clarity.
# Setup

docs/SNAP_OVERVIEW.md:1

  • Typo: 'paramters' should be 'parameters'. Also 'it's own state' should be 'its own state'.
# Overview

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dkonieczek dkonieczek requested review from Copilot and korgon October 2, 2025 15:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 77 out of 79 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (2)

doc-app.js:1

  • Headings are made interactive by click only and assigned role=link, but lack keyboard support and focusability. Add heading.tabIndex = 0 and a keydown handler (Enter/Space) that triggers the same action; include an aria-label (e.g., 'Copy permalink to clipboard') for screen readers.
function flattenDocumentLinks(docs) {

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dkonieczek dkonieczek requested a review from Copilot October 2, 2025 15:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 80 out of 82 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dkonieczek dkonieczek requested a review from Copilot October 2, 2025 15:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 80 out of 82 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dkonieczek
Copy link
Contributor Author

Copy link
Contributor

@korgon korgon left a comment

Choose a reason for hiding this comment

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

This is close - a few things I think we ought to resolve before publishing though, please see comments throughout.

Some of the code examples say .ts or .tsx, some .jsx and .js - can we standardize with using JS for now (as there is no typing in the examples)?

Examples in some of the documentation assume CDN integration scripts:

<script src="https://snapui.searchspring.io/[your_site_id]/bundle.js">

We should likely adjust this to use the element ID selector needed by getContext (Snap built in usage) and ditch the CDN URL and go with a simple bundle.js or similar.

For the "Snap Integration" section, seems we are missing the build/integration piece - I know it is in the "Reference" section - but feels like the section ends a bit too soon... Maybe there should be a shorter less technical section that discusses building a bundle vs. building into an existing project and then reference the build/deploy section in the reference?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the title in the sidebar could just be "Integration" under "Recommendations".

Image

Additionally, should both sections be shown to be active in the sidebar?

Image

@dkonieczek dkonieczek requested a review from korgon December 3, 2025 15:14
@korgon
Copy link
Contributor

korgon commented Dec 10, 2025

The recommendation controller appears to be missing the search method documentation.

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.

3 participants