-
Notifications
You must be signed in to change notification settings - Fork 326
fix: TimelineItem for default order #1884
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,7 @@ export const groupItem = tv({ | |
| img: "me-3 mb-3 w-12 h-12 rounded-full sm:mb-0", | ||
| div: "text-gray-600 dark:text-gray-400", | ||
| title: "text-base font-normal", | ||
| span: "inline-flex items-center text-xs font-normal text-gray-500 dark:text-gray-400", | ||
| span: "inline-flex items-center bg-gray-100 border border-gray-200 text-xs font-medium px-1.5 py-0.5 rounded", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify if groupItem span styling change is intentional. This line changes the Confirm whether this styling change is:
Run the following script to check if this change is referenced in any related issues or documentation: #!/bin/bash
# Description: Check for references to groupItem or badge styling changes
# Search for groupItem usage and styling references
echo "=== Searching for groupItem component usage ==="
rg -n "groupItem|GroupItem" --type=svelte --type=ts -C 2
# Search for any TODO or comments about badge styling
echo -e "\n=== Checking for badge-related comments ==="
rg -n "badge|pill" src/lib/timeline/ -C 2 |
||
| svg: "me-1 h-3 w-3" | ||
| } | ||
| }); | ||
|
|
@@ -109,6 +109,7 @@ export const timelineItem = tv({ | |
| slots: { | ||
| base: "relative", | ||
| div: "", | ||
| defaultDiv: "absolute w-2 h-2 bg-gray-200 rounded-full mt-1.5 -start-5 border border-buffer", | ||
| time: "", | ||
| h3: "", | ||
| svg: "w-4 h-4", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <script lang="ts"> | ||
| import type { PageProps } from "./$types"; | ||
|
|
||
| let { data }: PageProps = $props(); | ||
| let Content = $derived(data.content); | ||
| </script> | ||
|
|
||
| <Content /> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import type { PageLoad } from "./$types"; | ||
| import { error } from "@sveltejs/kit"; | ||
|
|
||
| export const load: PageLoad = async ({ params }) => { | ||
| try { | ||
| const post = await import(`../${params.slug}.svelte`); | ||
| const content = post.default; | ||
|
|
||
| return { | ||
| content, | ||
| }; | ||
| } catch (err) { | ||
| throw error(404, `Timeline example "${params.slug}" not found`); | ||
| } | ||
| }; | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Restore reactive context for dynamic order updates.
The direct
setContextcall with thesvelte-ignorecomment removes reactivity. If theorderprop changes after mount, child TimelineItem components won't receive the updated value.Restore the reactive effect pattern to ensure context updates propagate:
This ensures that when
orderchanges, all child components receive the updated context value.π Committable suggestion
π€ Prompt for AI Agents
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.
This doesn't work. It breaks to render unnecessary code.
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.
@shinokada, you're absolutely right, and I apologize for the incorrect suggestion. Using
$effectto repeatedly callsetContextcan indeed cause rendering issues.In Svelte 5,
setContextshould typically be called once at the component's initialization, not inside effects. The current implementation with the direct call is correct for this pattern.Could you clarify: Is the
orderprop expected to change dynamically after the Timeline component mounts, or is it set once during initialization and remains constant? This will help me understand the intended behavior and avoid similar incorrect suggestions in the future.π§ Learnings used