-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/one time token 2 #113
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
Conversation
WalkthroughThis change removes the sign-in and profile pages, their associated routes, and related navigation logic from the application. It deletes the accessibility tree processing module, updates navigation buttons to link externally for sign-in and account management, and adjusts some UI styling and placeholder elements. No new functional features are introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant ExternalSite
User->>App: Clicks "Sign In"
App->>ExternalSite: Opens https://getgrinta.com/sign-in in new tab
Note over App,ExternalSite: No internal sign-in route or profile page
sequenceDiagram
participant User
participant App
participant ExternalSite
User->>App: Clicks account/avatar button
App->>ExternalSite: Opens account or sign-in page in new tab (based on auth state)
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (2)
apps/extension/src/pages/settings.svelte (1)
95-104
: Smart Actions placeholder added without implementation.The new "Smart Actions" section contains disabled UI elements without functionality. This appears to be a placeholder for future features.
Consider adding a TODO comment with implementation details or adding a "Coming soon" label to set proper user expectations. If this is intended for a future release, documenting the planned functionality would help maintain context.
apps/extension/src/pages/home.svelte (1)
318-335
: External authentication flow replaces internal pages.The code replaces an internal settings navigation button with external links to authentication pages. This aligns with the removal of sign-in and profile pages mentioned in the PR summary.
However, there are a few considerations:
- The hardcoded URLs could make environment switching challenging
- The image URL contains many query parameters that might be better defined as constants
Consider extracting the URLs to constants or configuration:
+const ACCOUNT_URL = "https://getgrinta.com/account"; +const SIGN_IN_URL = "https://getgrinta.com/sign-in"; +const AVATAR_URL_BASE = "https://meshy.studio/api/mesh/"; +const AVATAR_PARAMS = "?noise=8&sharpen=1&negate=false&gammaIn=2.1&gammaOut=2.2&brightness=100&saturation=100&hue=0&lightness=0&blur=0"; // Then in the JSX: - href={user - ? "https://getgrinta.com/account" - : "https://getgrinta.com/sign-in"} + href={user ? ACCOUNT_URL : SIGN_IN_URL} // And for the avatar: - src={`https://meshy.studio/api/mesh/${user.id}?noise=8&sharpen=1&negate=false&gammaIn=2.1&gammaOut=2.2&brightness=100&saturation=100&hue=0&lightness=0&blur=0`} + src={`${AVATAR_URL_BASE}${user.id}${AVATAR_PARAMS}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/extension/src/app.svelte
(0 hunks)apps/extension/src/lib/ax.ts
(0 hunks)apps/extension/src/lib/components/layout.svelte
(2 hunks)apps/extension/src/lib/components/sidebar-tab-context-menu.svelte
(1 hunks)apps/extension/src/lib/components/space-essential-context-menu.svelte
(1 hunks)apps/extension/src/pages/chats.svelte
(1 hunks)apps/extension/src/pages/home.svelte
(5 hunks)apps/extension/src/pages/profile.svelte
(0 hunks)apps/extension/src/pages/settings.svelte
(2 hunks)apps/extension/src/pages/sign-in.svelte
(0 hunks)
💤 Files with no reviewable changes (4)
- apps/extension/src/app.svelte
- apps/extension/src/pages/sign-in.svelte
- apps/extension/src/pages/profile.svelte
- apps/extension/src/lib/ax.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/extension/src/lib/components/space-essential-context-menu.svelte (1)
27-27
: UI update looks good.The background color change from
bg-base-200
tobg-base-300
aligns with similar updates in other context menus, maintaining visual consistency across the application.apps/extension/src/lib/components/sidebar-tab-context-menu.svelte (1)
65-65
: UI update looks good.The background color change from
bg-base-200
tobg-base-300
maintains visual consistency with other context menus in the application, including the space-essential-context-menu component.apps/extension/src/pages/chats.svelte (1)
289-293
: Sign-in navigation change properly implemented.Changed the internal navigation button to an external link to the sign-in page, consistent with removing internal sign-in and profile pages from the application. The external link correctly includes security attributes (
target="_blank"
andrel="noopener noreferrer"
) to prevent potential security issues when opening in a new tab.apps/extension/src/lib/components/layout.svelte (2)
5-10
: Icon import update looks good.Added
SettingsIcon
to the imports, which is used in the updated navigation button.
57-62
: Navigation change properly implemented.The button for profile/sign-in navigation has been replaced with a settings navigation button:
- Updated the active state logic to check for "/settings" instead of "/profile" or "/sign-in"
- Changed the navigation target to "/settings" unconditionally
- Added appropriate aria-label for accessibility
- Replaced the conditional user avatar/icon with the SettingsIcon
This change aligns with the removal of sign-in and profile pages from the application.
apps/extension/src/pages/settings.svelte (1)
72-94
:✅ Verification successful
Confirm persistence strategy for spaces configuration.
The form for spaces no longer has an
onchange
handler. Previously, this likely triggeredonSpacesChanged
to persist space modifications. Verify that space changes are still properly saved, perhaps through the individual element handlers.
🏁 Script executed:
#!/bin/bash # Check if space changes are persisted elsewhere in the codebase rg -A 3 "persist.*space|store.*space" --glob "*.{js,ts,svelte}"Length of output: 65
🏁 Script executed:
#!/bin/bash set -e # Locate updateGroupTitle usage and definition echo "Search for updateGroupTitle references:" rg -n "updateGroupTitle" apps/extension/src/pages/settings.svelte || echo "No matches for updateGroupTitle in settings.svelte" echo # Locate deleteGroup usage and definition echo "Search for deleteGroup references:" rg -n "deleteGroup" apps/extension/src/pages/settings.svelte || echo "No matches for deleteGroup in settings.svelte" echo # Show imports and function definitions in the file for context echo "Top 200 lines of settings.svelte:" sed -n '1,200p' apps/extension/src/pages/settings.svelteLength of output: 3938
🏁 Script executed:
#!/bin/bash set -e # Find the file defining tabsStore echo "Locating tabsStore definition:" rg -l "tabsStore" -g "*.{ts,svelte}" || echo "tabsStore definition not found" echo # Assuming the store is in the found file, display its contents TAB_STORE_FILE=$(rg -l "tabsStore" -g "*.{ts,svelte}" | head -n1) if [ -n "$TAB_STORE_FILE" ]; then echo "Showing contents of $TAB_STORE_FILE:" sed -n '1,200p' "$TAB_STORE_FILE" else echo "Could not locate tabsStore file." fiLength of output: 2778
🏁 Script executed:
#!/bin/bash set -e # Display the contents of the tabsStore to inspect persistence and state sync logic TABS_STORE_FILE="apps/extension/src/lib/store/tabs.svelte.ts" if [ -f "$TABS_STORE_FILE" ]; then echo "Showing first 200 lines of $TABS_STORE_FILE:" sed -n '1,200p' "$TABS_STORE_FILE" echo echo "Showing lines 201-400 of $TABS_STORE_FILE:" sed -n '201,400p' "$TABS_STORE_FILE" else echo "tabsStore file not found at $TABS_STORE_FILE" fiLength of output: 1841
Space changes persist via individual handlers
The
updateGroupTitle
anddeleteGroup
functions each invokesendMessage
to the background to save updates and removals immediately. Removing the form-levelonchange
handler does not affect persistence, as every change is handled at the input/button level.apps/extension/src/pages/home.svelte (1)
59-60
: LGTM: Authentication integration looks good.The authentication state is properly managed using
authClient.useSession()
with the user object correctly derived from session data.Also applies to: 70-71
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores