feat: add margin notes by JasonLovesDoggo · Pull Request #21 · JasonLovesDoggo/nyx

JasonLovesDoggo · 2025-12-27T22:53:06Z

No description provided.

…age.svelte

cloudflare-workers-and-pages · 2025-12-27T22:53:14Z

Deploying nyx with Cloudflare Pages

Latest commit:	`b49e815`
Status:	✅ Deploy successful!
Preview URL:	https://269c2780.nyx-2cw.pages.dev
Branch Preview URL:	https://feat-marginnotes.nyx-2cw.pages.dev

View logs

Copilot

Pull request overview

This PR implements margin notes (also called sidenotes) for blog posts, allowing inline annotations that expand on mobile and appear in the margin on wider screens. The implementation uses Svelte's context API to manage note numbering and includes responsive styling.

Creates a reusable Margin component with toggle functionality for mobile and persistent display for desktop
Implements a context-based counter system to automatically number margin notes sequentially
Updates post page layout to accommodate margin notes with adjusted spacing and positioning

Reviewed changes

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

Show a summary per file

File	Description
`src/lib/stores/sidenote.ts`	New store implementing Svelte context for sequential numbering of margin notes
`src/components/posts/Margin.svelte`	New component for rendering margin notes with responsive behavior (inline toggle on mobile, margin display on desktop)
`src/routes/posts/[slug]/+page.svelte`	Initializes margin note counter, updates layout constraints to support margin positioning, adds post footer
`src/routes/about/+page.svelte`	Removes commented-out code and local scroll-padding-top style in favor of global implementation
`src/app.css`	Adds global scroll-padding-top for anchor navigation, normalizes quote style in @plugin directive
`content/posts/agno-stop-button.svx`	Demonstrates margin note usage with explanatory note about ASGI in table cell

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/components/posts/Margin.svelte

src/lib/stores/sidenote.ts

Copilot · 2025-12-27T22:55:32Z

src/components/posts/Margin.svelte

+	}
+
+	.sidenote.expanded {
+		max-height: 500px;


The hardcoded max-height value of 500px may cause content truncation for longer margin notes. Consider using a larger value or calculating the height dynamically to ensure all content is visible when expanded.

src/lib/stores/sidenote.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

# Conflicts: # src/components/posts/Margin.svelte

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-27T23:06:06Z

src/routes/posts/[slug]/+page.svelte

 	</header>

-	<article class="prose mx-auto mb-6 max-w-4xl">
+	<article class="prose relative mb-6 max-w-none overflow-visible px-4">


The article now uses "max-w-none" instead of "max-w-4xl", which removes the maximum width constraint. This could cause the content to become uncomfortably wide on large screens. Consider keeping a reasonable max-width for better readability while still allowing space for margin notes, or use a more specific container strategy.

Copilot · 2025-12-27T23:06:07Z

content/posts/agno-stop-button.svx

 | Method               | Use Case                              | Pros                        | Cons                           |
 |----------------------|---------------------------------------|-----------------------------|--------------------------------|
-| HTTP Disconnect      | FastAPI streaming endpoints           | Simple, no infra needed     | Relies on ASGI behavior        |
+| HTTP Disconnect      | FastAPI streaming endpoints           | Simple, no infra needed     | Relies on ASGI behavior<Margin>ASGI (Asynchronous Server Gateway Interface) is Python's async web server spec. Breaking down ASGI vs WSGI vs WGSI vs RSGI is a whole other topic.</Margin>        |


Typo in the margin note: "WGSI" should be "WSGI" (Web Server Gateway Interface). The correct acronym sequence should be "ASGI vs WSGI vs RSGI".

Suggested change

| HTTP Disconnect | FastAPI streaming endpoints | Simple, no infra needed | Relies on ASGI behavior<Margin>ASGI (Asynchronous Server Gateway Interface) is Python's async web server spec. Breaking down ASGI vs WSGI vs WGSI vs RSGI is a whole other topic.</Margin> |

| HTTP Disconnect | FastAPI streaming endpoints | Simple, no infra needed | Relies on ASGI behavior<Margin>ASGI (Asynchronous Server Gateway Interface) is Python's async web server spec. Breaking down ASGI vs WSGI vs RSGI is a whole other topic.</Margin> |

Copilot · 2025-12-27T23:06:07Z

src/components/posts/Margin.svelte

+	<span class="marginnote" class:expanded
+		><span class="marginnote-number">[{number}]</span>{@render children()}</span
+	>


The margin note content in the expanded state should have an id attribute that corresponds to the aria-expanded button, following proper ARIA relationships. Consider adding an id to the margin note span and using aria-controls on the button to link them together for better accessibility.

Copilot · 2025-12-27T23:06:07Z

src/components/posts/Margin.svelte

+		.marginnote {
+			display: block;
+			position: absolute;
+			right: 0;
+			top: auto;
+			transform: translateX(calc(100% + 1rem));
+			width: 200px;


The absolute positioning of margin notes on desktop (line 94) relies on the parent having position: relative. However, the margin notes may not align correctly if they appear within nested elements inside the article. Consider documenting this requirement or using a more robust positioning strategy like CSS Grid or Flexbox at the article level.

Copilot · 2025-12-27T23:06:07Z

src/components/posts/Margin.svelte

+	.marginnote.expanded {
+		max-height: 500px;


Using max-height for transitions with a large fixed value (500px) can cause janky animations when the actual content is much smaller. Consider using a more precise max-height value based on the actual content height, or use a different animation approach like CSS grid animations or the View Transitions API for smoother performance.

Copilot · 2025-12-27T23:06:08Z

src/lib/stores/marginnote.ts

+		console.warn('Margin note used outside of initMarginNoteCounter context');
+		return 0;


Returning 0 when the context is not found could lead to multiple margin notes having the same number (0) if initMarginNoteCounter() is not called properly. Consider throwing an error instead to make this failure mode more visible during development, or return a unique identifier to prevent duplicate numbers.

Suggested change

console.warn('Margin note used outside of initMarginNoteCounter context');

return 0;

throw new Error('Margin note used outside of initMarginNoteCounter context');

JasonLovesDoggo added 2 commits December 27, 2025 17:39

feat: implement sidenote functionality with counter and toggle behavior

2f7b289

feat: integrate Margin component into agno-stop-button and clean up p…

6fa2588

…age.svelte

Copilot AI review requested due to automatic review settings December 27, 2025 22:53

Merge branch 'main' into feat/marginnotes

50512e2

Copilot started reviewing on behalf of JasonLovesDoggo December 27, 2025 22:53 View session

Copilot AI reviewed Dec 27, 2025

View reviewed changes

JasonLovesDoggo and others added 2 commits December 27, 2025 17:57

Update src/components/posts/Margin.svelte

8f90215

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

feat: refactor sidenote functionality to margin note implementation

b49e815

# Conflicts: # src/components/posts/Margin.svelte

JasonLovesDoggo requested a review from Copilot December 27, 2025 23:02

Copilot started reviewing on behalf of JasonLovesDoggo December 27, 2025 23:03 View session

Copilot AI reviewed Dec 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add margin notes#21

feat: add margin notes#21
JasonLovesDoggo wants to merge 5 commits intomainfrom
feat/marginnotes

JasonLovesDoggo commented Dec 27, 2025

Uh oh!

cloudflare-workers-and-pages bot commented Dec 27, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Copilot AI Dec 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	\| HTTP Disconnect \| FastAPI streaming endpoints \| Simple, no infra needed \| Relies on ASGI behavior<Margin>ASGI (Asynchronous Server Gateway Interface) is Python's async web server spec. Breaking down ASGI vs WSGI vs WGSI vs RSGI is a whole other topic.</Margin> \|
	\| HTTP Disconnect \| FastAPI streaming endpoints \| Simple, no infra needed \| Relies on ASGI behavior<Margin>ASGI (Asynchronous Server Gateway Interface) is Python's async web server spec. Breaking down ASGI vs WSGI vs RSGI is a whole other topic.</Margin> \|

		console.warn('Margin note used outside of initMarginNoteCounter context');
		return 0;

	console.warn('Margin note used outside of initMarginNoteCounter context');
	return 0;
	throw new Error('Margin note used outside of initMarginNoteCounter context');

Conversation

JasonLovesDoggo commented Dec 27, 2025

Uh oh!

cloudflare-workers-and-pages bot commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying nyx with Cloudflare Pages

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cloudflare-workers-and-pages bot commented Dec 27, 2025 •

edited

Loading